mgechev / mgechev/codelyzer

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

Open
#530 2 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

bug
Dominant language
TypeScript
Stars
2.5k
Forks
232
PR merge metrics
No merged PRs in 30d

Description

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 npm@5.3.0
npm info using node@v8.5.0
npm info lifecycle codelyzer@4.2.1~pretest: codelyzer@4.2.1
npm info lifecycle codelyzer@4.2.1~test: codelyzer@4.2.1

> codelyzer@4.2.1 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 codelyzer@4.2.1~test: Failed to exec test script
npm ERR! code ELIFECYCLE
npm ERR! errno 1
npm ERR! codelyzer@4.2.1 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 codelyzer@4.2.1 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"]

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Run the repository's npm run test command and inspect the i18n check-id tests, including the failing ng-container case and the passing span case shown in dist/test/i18nRule.spec.js. Trace the rule behavior for check-id and check-text; done means the ng-container i18n="@@foo" test passes without regressing the existing cases.

Written by the indexing model from the issue text.

Assessment

Tech stack
angular, typescript
Domain
tooling
Issue type
Bug
Difficulty
3/5
Estimated time
1-2 days
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
35/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.