Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

--fix is broken with .gts files #2681

Closed
NullVoxPopuli opened this issue Nov 5, 2022 · 7 comments · Fixed by #2696
Closed

--fix is broken with .gts files #2681

NullVoxPopuli opened this issue Nov 5, 2022 · 7 comments · Fixed by #2696
Labels

Comments

@NullVoxPopuli
Copy link
Contributor

NullVoxPopuli commented Nov 5, 2022

Here is where I am testing: NullVoxPopuli/limber#545

cd frontend
pnpm i
pnpm lint:hbs
pnpm lint:hbs --fix

Without --fix works fine:

❯  pnpm ember-template-lint app/components/limber/output/index.gts 
app/components/limber/output/index.gts
  16:10  error  Delete `⏎··`  prettier
  17:58  error  Delete `··`  prettier
  19:4  error  Replace `··{{!--` with `{{!`  prettier
  24:6  error  Delete `--`  prettier
  24:11  error  Delete `··`  prettier
  26:1  error  Replace `······` with `    `  prettier
  27:32  error  Delete `··`  prettier
  28:60  error  Replace `··········{{!--·@glint-ignore·--` with `        {{! @glint-ignore `  prettier
  29:35  error  Delete `··`  prettier
  31:10  error  Delete `··`  prettier
  31:24  error  Delete `··`  prettier
  33:6  error  Delete `··`  prettier
  33:17  error  Replace `······` with `    `  prettier
  35:1  error  Replace `····` with `  `  prettier
  36:11  error  Replace `··</Compiler>⏎` with `</Compiler>`  prettier

With fix gives an error an the typescript syntax:

❯  pnpm ember-template-lint app/components/limber/output/index.gts --fix
/home/nullvoxpopuli/Development/NullVoxPopuli/limber/node_modules/.pnpm/@glimmer+syntax@0.83.1/node_modules/@glimmer/syntax/dist/commonjs/es2017/lib/syntax-error.js:19
let error = new Error(`${message}: ${quotedCode}(error occurred in '${module}' @ line ${line} : column ${column})`);
            ^

Error [SyntaxError]: Unclosed element `Signature`: 

|
|  <Signature>
|

(error occurred in 'an unknown module' @ line 15 : column 24)
  at generateSyntaxError (/home/nullvoxpopuli/Development/NullVoxPopuli/limber/node_modules/.pnpm/@glimmer+syntax@0.83.1/node_modules/@glimmer/syntax/dist/commonjs/es2017/lib/syntax-error.js:19:15)
  at TokenizerEventHandlers.Program (/home/nullvoxpopuli/Development/NullVoxPopuli/limber/node_modules/.pnpm/@glimmer+syntax@0.83.1/node_modules/@glimmer/syntax/dist/commonjs/es2017/lib/parser/handlebars-node-visitors.js:63:50)
  at TokenizerEventHandlers.acceptTemplate (/home/nullvoxpopuli/Development/NullVoxPopuli/limber/node_modules/.pnpm/@glimmer+syntax@0.83.1/node_modules/@glimmer/syntax/dist/commonjs/es2017/lib/parser.js:78:27)
  at preprocess (/home/nullvoxpopuli/Development/NullVoxPopuli/limber/node_modules/.pnpm/@glimmer+syntax@0.83.1/node_modules/@glimmer/syntax/dist/commonjs/es2017/lib/parser/tokenizer-event-handlers.js:377:72)
  at new ParseResult (/home/nullvoxpopuli/Development/NullVoxPopuli/limber/node_modules/.pnpm/ember-template-recast@6.1.3/node_modules/ember-template-recast/lib/parse-result.js:101:43)

so, maybe somehow when in fix mode, the templates aren't getting parsed / extracted appropriately.

looks like the same error on gjs:

 pnpm lint:hbs app/components/limber/output/js-test.gjs  --fix

> limber@0.0.0 lint:hbs /home/nullvoxpopuli/Development/NullVoxPopuli/limber/frontend
> ember-template-lint . --no-error-on-unmatched-pattern "app/components/limber/output/js-test.gjs" "--fix"

/home/nullvoxpopuli/Development/NullVoxPopuli/limber/node_modules/.pnpm/@glimmer+syntax@0.83.1/node_modules/@glimmer/syntax/dist/commonjs/es2017/lib/syntax-error.js:19
  let error = new Error(`${message}: ${quotedCode}(error occurred in '${module}' @ line ${line} : column ${column})`);
              ^

Error [SyntaxError]: ' is not a valid character within attribute names: (error occurred in 'an unknown module' @ line 16 : column 12)
    at generateSyntaxError (/home/nullvoxpopuli/Development/NullVoxPopuli/limber/node_modules/.pnpm/@glimmer+syntax@0.83.1/node_modules/@glimmer/syntax/dist/commonjs/es2017/lib/syntax-error.js:19:15)
    at TokenizerEventHandlers.reportSyntaxError (/home/nullvoxpopuli/Development/NullVoxPopuli/limber/node_modules/.pnpm/@glimmer+syntax@0.83.1/node_modules/@glimmer/syntax/dist/commonjs/es2017/lib/parser/tokenizer-event-handlers.js:257:48)
    at EventedTokenizer.attributeName (/home/nullvoxpopuli/Development/NullVoxPopuli/limber/node_modules/.pnpm/simple-html-tokenizer@0.5.11/node_modules/simple-html-tokenizer/dist/simple-html-tokenizer.js:473:39)
    at EventedTokenizer.tokenizePart (/home/nullvoxpopuli/Development/NullVoxPopuli/limber/node_modules/.pnpm/simple-html-tokenizer@0.5.11/node_modules/simple-html-tokenizer/dist/simple-html-tokenizer.js:661:29)
    at TokenizerEventHandlers.ContentStatement (/home/nullvoxpopuli/Development/NullVoxPopuli/limber/node_modules/.pnpm/@glimmer+syntax@0.83.1/node_modules/@glimmer/syntax/dist/commonjs/es2017/lib/parser/handlebars-node-visitors.js:254:20)
    at TokenizerEventHandlers.acceptNode (/home/nullvoxpopuli/Development/NullVoxPopuli/limber/node_modules/.pnpm/@glimmer+syntax@0.83.1/node_modules/@glimmer/syntax/dist/commonjs/es2017/lib/parser.js:82:27)
    at TokenizerEventHandlers.Program (/home/nullvoxpopuli/Development/NullVoxPopuli/limber/node_modules/.pnpm/@glimmer+syntax@0.83.1/node_modules/@glimmer/syntax/dist/commonjs/es2017/lib/parser/handlebars-node-visitors.js:55:12)
    at TokenizerEventHandlers.acceptTemplate (/home/nullvoxpopuli/Development/NullVoxPopuli/limber/node_modules/.pnpm/@glimmer+syntax@0.83.1/node_modules/@glimmer/syntax/dist/commonjs/es2017/lib/parser.js:78:27)
    at preprocess (/home/nullvoxpopuli/Development/NullVoxPopuli/limber/node_modules/.pnpm/@glimmer+syntax@0.83.1/node_modules/@glimmer/syntax/dist/commonjs/es2017/lib/parser/tokenizer-event-handlers.js:377:72)
    at new ParseResult (/home/nullvoxpopuli/Development/NullVoxPopuli/limber/node_modules/.pnpm/ember-template-recast@6.1.3/node_modules/ember-template-recast/lib/parse-result.js:101:43)
@NullVoxPopuli NullVoxPopuli changed the title --fix breaks on gts [5 alpha.0] --fix breaks on gts/gjs Nov 5, 2022
@bmish bmish added the bug label Nov 5, 2022
@bmish
Copy link
Member

bmish commented Nov 5, 2022

Thanks for reporting. Does this block the v5 release (#2319)? Do you expect to be able to fix it? I'm hoping to get that release out in the next week or so.

@NullVoxPopuli
Copy link
Contributor Author

Since Polaris isn't default yet, I don't think this would be blocking.
I intend to look at this, but I don't want to make any promises -- so much to do 🙃

@rwjblue
Copy link
Member

rwjblue commented Nov 7, 2022

Looks like the Linter.prototype.verifyAndFix pathway didn’t get the same treatment as Linter.prototype.verify (migration to use parseTemplates).

async verifyAndFix(options) {

@rwjblue
Copy link
Member

rwjblue commented Nov 7, 2022

@bmish - This is definitely a bug (and probably a pretty annoying one for folks using GJS / GTS), but since it’s 1) clearly a bug and 2) only affects folks using syntax that was not supported by default in 4.x, it probably doesn’t need to block 5.0.0.

However, fixing it probably isn’t mega-hard…

@rwjblue rwjblue changed the title [5 alpha.0] --fix breaks on gts/gjs --fix is broken with .gts files Nov 7, 2022
@bertdeblock
Copy link
Collaborator

Since v5 will include .js files by default, this could/will also affect .js files with hbs tags, for example, integration test files. When testing with ember-template-lint-plugin-prettier, the entire file is formatted, and not only the templates inside the file.

@rwjblue
Copy link
Member

rwjblue commented Nov 7, 2022

Ya, good point!

Minor side point:

Since v5 will include .js files by default, this could/will also affect .js files with hbs tags, for example, integration test files.

The particular issue reported here (unclosed elements) is likely to only affect TS files (because generics look like unclosed elements to the parser), but you are 100% correct that other problems will likely crop up.

@rwjblue
Copy link
Member

rwjblue commented Nov 8, 2022

I created some simple test case failures for this issue over in #2696

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants