fix: no-unlocalized-strings rule to ignore string literals in expressions assigned to variables specified in ignoreNames

[swernerx]

Description:

This PR addresses an issue in the no-unlocalized-strings ESLint rule where string literals within expressions assigned to variables are incorrectly reported as unlocalized strings, even when the variable names are specified in the ignoreNames option.

Background:

When using the ignoreNames option to ignore certain variable names, the rule should not report string literals assigned to those variables. However, if the string literal is part of an expression (e.g., using the nullish coalescing operator ??, logical OR ||, or ternary ? :), the rule currently fails to recognize it and reports an error.

Example:

Given the ESLint configuration:

{
  "rules": {
    "no-unlocalized-strings": [
      "error",
      { "ignoreNames": ["variant"] }
    ]
  }
}

The following code incorrectly reports an error for the string literals "body", "yes", and "no":

const variant = input ?? "body";
const variant = input || "body";
const variant = condition ? "yes" : "no";

Cause of the Issue:

The rule does not currently check whether a string literal is part of an expression assigned to a variable specified in ignoreNames. It only handles direct assignments of string literals.

Solution:

To fix this issue, I modified the 'Literal:exit' and 'TemplateLiteral:exit' handlers in the rule to traverse up the Abstract Syntax Tree (AST) and check if the literal is part of a VariableDeclarator or AssignmentExpression where the variable name matches an entry in ignoreNames. If it is, the rule will ignore the literal and not report an error.

Explanation:

• Traverse Up the AST: The handlers now traverse up the AST from the literal node to check if it's part of a variable declaration or assignment where the variable name matches an entry in ignoreNames.
• Check Variable Names: If a matching variable name is found, the handler returns early, and the literal is not reported.
• Minimal Changes: This fix is minimal and does not introduce new visitor functions or complex recursion, reducing the risk of regressions.

Testing:

• Manual Testing:

  • Verified that string literals in expressions assigned to variables specified in ignoreNames are no longer reported.
  • Tested with various expressions, including nullish coalescing (??), logical OR (||), and ternary (? :) operators.

• Automated Testing:

  • Ran the existing test suite to ensure no regressions were introduced.
  • Added new test cases to cover scenarios involving string literals within expressions assigned to variables in ignoreNames.

Conclusion:

This PR improves the no-unlocalized-strings rule by ensuring that string literals within expressions assigned to variables specified in ignoreNames are correctly ignored, aligning the rule's behavior with user expectations and the documentation.

[codecov]

Codecov Report

Attention: Patch coverage is 98.36066% with 1 line in your changes missing coverage. Please review.

Project coverage is 97.18%. Comparing base (4048c4d) to head (0144b62).
Report is 2 commits behind head on main.

Files with missing lines	Patch %	Lines
src/rules/no-unlocalized-strings.ts	98.36%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main      #94      +/-   ##
==========================================
+ Coverage   96.60%   97.18%   +0.58%     
==========================================
  Files          10       10              
  Lines         383      427      +44     
  Branches      105      131      +26     
==========================================
+ Hits          370      415      +45     
+ Misses         13       12       -1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[timofei-iatsenko]

looks like there is a code duplication which could be extracted into a reusable function.

Also please add the same set of test where TemplateLiteral is used all these cases.

[swernerx]

I have refactored the code to extract the logic and also added tests for template literals. This also enhanced code coverage.

[timofei-iatsenko]

it seems that might also cover the cases where something more complex is assigned to variable

{
      code: 'const variant = myFunction({someProperty: "Hello World!"})',
      options: [{ ignoreNames: ['variant'] }],
    },

Could you test it locally, am I correct with that?

PS. I could not say is it good or bad. If it's also covering this, for some users that might be confusing, because "why argument of the myFunction is not flagged, it's not in the ignoreFunction and someProperty is not in the ignoreNames"

From other side it still might be good, i recently had such case, don't remember exactly what it was, but I wished that this behavior would be there.

[swernerx]

I guess this is indeed not good. I would expect to receive to get an error for someProperty. We have such code quite a bit with data structures for menus which also contain e.g. labels.

[swernerx]

I pushed another rework of the code to prevent these cases. I also added the check to the error section of the test suite.

[swernerx]

I made some adjustments to the handling of JSX strings. Based on my understanding, expressions like <Foo>{'hello'}</Foo> qualify as JSX text and should ideally be reported as such rather than falling back to the generic "default" label. This change ensures a more accurate representation of JSX elements in these cases.

Additionally, I added istanbul ignore next directives to sections of code that primarily handle the default behavior of functions. These parts are currently not covered by tests, but their lack of coverage is intentional and well-documented. Using these directives helps avoid unnecessary coverage reports while maintaining clarity and safety.

[swernerx]

I applied some more tests from issues I have seen in our code base. Looks good for existing test cases.

[timofei-iatsenko]

LGTM

[andrii-bodnar]

@swernerx thanks for the contribution, and @timofei-iatsenko thanks for the review!