codelyzer icon indicating copy to clipboard operation
codelyzer copied to clipboard

i18n rule with check-id and check-text does not handle the ng-container tag

Open sgallen opened this issue 6 years ago • 2 comments

Reference: PR #506

Today I updated to version 4.2.1 of Codelyzer, as I was finding the linter was incorrectly reporting errors with our use of select ICU expressions (see: https://angular.io/guide/i18n#translate-select). PR #506 seems to have addressed those issues (thanks!). Unfortunately, I'm now finding that when I use ng-container tags with i18n ids (see: https://angular.io/guide/i18n#translate-text-without-creating-an-element) I get linting errors, that I don't expect.

I've cloned the repo, locally adding 2 tests to the suite. They're identical tests except for the use of different tags (ng-container vs span). As you'll see further below ng-container fails while span works fine:

 66     it('NEW [ng-container]: should work with proper id', () => {
 67       const source = `
 68       @Component({
 69         template: \`
 70           <ng-container i18n="@@foo">Text</ng-container>
 71         \`
 72       })
 73       class Bar {}
 74       `;
 75       assertSuccess('i18n', source, ['check-id', 'check-text']);
 76     });
 77
 78     it('NEW [span]: should work with proper id', () => {
 79       const source = `
 80       @Component({
 81         template: \`
 82           <span i18n="@@foo">Text</span>
 83         \`
 84       })
 85       class Bar {}
 86       `;
 87       assertSuccess('i18n', source, ['check-id', 'check-text']);
 88     });

This is the output:

node@3bb72aa7d7e3:/codelyzer$ npm run test
npm info it worked if it ends with ok
npm info using [email protected]
npm info using [email protected]
npm info lifecycle [email protected]~pretest: [email protected]
npm info lifecycle [email protected]~test: [email protected]

> [email protected] test /codelyzer
> rimraf dist && tsc && cp -r test/fixtures dist/test && mocha dist/test --recursive



  i18n
    check-id
      ✓ should work with proper id (50ms)
      ✓ should work with proper i18n attribute
      ✓ should work with proper id
      ✓ should work with proper id
      1) NEW [ng-container]: should work with proper id
      ✓ NEW [span]: should work with proper id
      ✓ should fail with missing id string
      ✓ should fail with missing id
      ✓ should fail with missing id


  8 passing (130ms)
  1 failing

  1) i18n check-id NEW [ng-container]: should work with proper id:
     AssertionError: expected false to be true
      at Function.assert.isTrue (node_modules/chai/lib/chai/interface/assert.js:332:31)
      at Object.assertSuccess (dist/test/testHelper.js:161:17)
      at Context.<anonymous> (dist/test/i18nRule.spec.js:30:26)



npm info lifecycle [email protected]~test: Failed to exec test script
npm ERR! code ELIFECYCLE
npm ERR! errno 1
npm ERR! [email protected] test: `rimraf dist && tsc && cp -r test/fixtures dist/test && mocha dist/test --recursive`
npm ERR! Exit status 1
npm ERR!
npm ERR! Failed at the [email protected] test script.
npm ERR! This is probably not a problem with npm. There is likely additional logging output above.

npm ERR! A complete log of this run can be found in:
npm ERR!     /home/node/.npm/_logs/2018-03-04T17_39_02_030Z-debug.log

One caveat, is that I don't see any tests in which both check-id and check-text are set so it's possible I shouldn't be configuring the linter to use both. Not sure. This is the line that I have in my tslint file: "i18n": [true, "check-id", "check-text"]

sgallen avatar Mar 04 '18 17:03 sgallen

I believe @wKoza worked on this https://github.com/mgechev/codelyzer/pull/506/files

mgechev avatar Mar 04 '18 19:03 mgechev

Hi, That's what I feared . it is going to be difficult to distinguish ng-container declared by developer and generated by Angular.

wKoza avatar Mar 06 '18 20:03 wKoza