feat: add `no-useless-assignment` rule #17625

ota-meshi · 2023-10-07T07:05:25Z

Prerequisites checklist

I have read the contributing guidelines.

What is the purpose of this pull request? (put an "X" next to an item)

[ ] Documentation update
[ ] Bug fix (template)
[X] New rule (template)
[ ] Changes an existing rule (template)
[ ] Add autofix to a rule
[ ] Add a CLI option
[ ] Add something to the core
[ ] Other, please explain:

What changes did you make? (Give an overview)

This PR adds a new no-useless-assignment rule which warns if a variable is not used after being assigned a value.

close #17559

Is there anything you'd like reviewers to focus on?

~~This PR is still a draft as many test cases are missing.~~

netlify · 2023-10-07T07:05:29Z

✅ Deploy Preview for docs-eslint ready!

Name	Link
🔨 Latest commit	`f616420`
🔍 Latest deploy log	https://app.netlify.com/sites/docs-eslint/deploys/659d02b4404a2d0008f4050a
😎 Deploy Preview	https://deploy-preview-17625--docs-eslint.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

lib/rules/no-useless-assignment.js

tests/lib/rules/no-useless-assignment.js

lib/rules/no-useless-assignment.js

Co-authored-by: Nicholas C. Zakas <nicholas@humanwhocodes.com>

ota-meshi · 2023-10-20T01:21:40Z

Hmm. I don't understand why it crashes in CI. I can't reproduce it well locally. If you know the cause, could you please advise?

mdjermanovic · 2023-10-20T16:17:56Z

Hmm. I don't understand why it crashes in CI. I can't reproduce it well locally. If you know the cause, could you please advise?

The problem is fixed in dependencies now (#17664 (comment), fixed by humanwhocodes/object-schema@6ca80b0).

mdjermanovic · 2023-10-23T12:42:54Z

lib/rules/no-useless-assignment.js

+                    // It is unused variable
+                    return;


This is to avoid reporting on variables that are reported by no-unused-vars?

I thought it shouldn't be reported in the new rule because it's not an unnecessary assignment, it's clearly an unnecessary variable. And it is reported by no-unused-vars.

Can you add those details both as a comment here and in the documentation? I can see this being a bit confusing, but I agree with your rationale.

mdjermanovic · 2023-10-23T13:04:33Z

I tried enabling this rule in the eslint codebase (this branch) and it found 46 errors:

C:\projects\tmp\ota-meshi-eslint\Makefile.js
  375:9  error  This assigned value is not used in subsequent statements  no-useless-assignment
  388:9  error  This assigned value is not used in subsequent statements  no-useless-assignment

C:\projects\tmp\ota-meshi-eslint\lib\linter\code-path-analysis\code-path.js
  183:13  error  This assigned value is not used in subsequent statements  no-useless-assignment
  184:13  error  This assigned value is not used in subsequent statements  no-useless-assignment
  185:13  error  This assigned value is not used in subsequent statements  no-useless-assignment

C:\projects\tmp\ota-meshi-eslint\lib\linter\config-comment-parser.js
   75:13  error  This assigned value is not used in subsequent statements  no-useless-assignment
  102:9   error  This assigned value is not used in subsequent statements  no-useless-assignment

C:\projects\tmp\ota-meshi-eslint\lib\linter\linter.js
  2062:13  error  This assigned value is not used in subsequent statements  no-useless-assignment

C:\projects\tmp\ota-meshi-eslint\lib\linter\timing.js
  140:17  error  This assigned value is not used in subsequent statements  no-useless-assignment

C:\projects\tmp\ota-meshi-eslint\lib\rules\array-bracket-newline.js
  74:17  error  This assigned value is not used in subsequent statements  no-useless-assignment

C:\projects\tmp\ota-meshi-eslint\lib\rules\indent-legacy.js
  712:17  error  This assigned value is not used in subsequent statements  no-useless-assignment
  833:17  error  This assigned value is not used in subsequent statements  no-useless-assignment

C:\projects\tmp\ota-meshi-eslint\lib\rules\indent.js
  1407:21  error  This assigned value is not used in subsequent statements  no-useless-assignment

C:\projects\tmp\ota-meshi-eslint\lib\rules\key-spacing.js
  489:21  error  This assigned value is not used in subsequent statements  no-useless-assignment

C:\projects\tmp\ota-meshi-eslint\lib\rules\lines-around-comment.js
  156:17  error  This assigned value is not used in subsequent statements  no-useless-assignment
  166:13  error  This assigned value is not used in subsequent statements  no-useless-assignment

C:\projects\tmp\ota-meshi-eslint\lib\rules\max-len.js
  344:25  error  This assigned value is not used in subsequent statements  no-useless-assignment

C:\projects\tmp\ota-meshi-eslint\lib\rules\max-lines.js
  112:13  error  This assigned value is not used in subsequent statements  no-useless-assignment
  123:13  error  This assigned value is not used in subsequent statements  no-useless-assignment

C:\projects\tmp\ota-meshi-eslint\lib\rules\no-dupe-class-members.js
  85:21  error  This assigned value is not used in subsequent statements  no-useless-assignment

C:\projects\tmp\ota-meshi-eslint\lib\rules\no-dupe-else-if.js
  103:21  error  This assigned value is not used in subsequent statements  no-useless-assignment

C:\projects\tmp\ota-meshi-eslint\lib\rules\no-empty-function.js
  43:9  error  This assigned value is not used in subsequent statements  no-useless-assignment
  76:9  error  This assigned value is not used in subsequent statements  no-useless-assignment

C:\projects\tmp\ota-meshi-eslint\lib\rules\no-extra-parens.js
  497:17  error  This assigned value is not used in subsequent statements  no-useless-assignment

C:\projects\tmp\ota-meshi-eslint\lib\rules\no-loss-of-precision.js
  67:17  error  This assigned value is not used in subsequent statements  no-useless-assignment

C:\projects\tmp\ota-meshi-eslint\lib\rules\no-object-constructor.js
  64:9  error  This assigned value is not used in subsequent statements  no-useless-assignment

C:\projects\tmp\ota-meshi-eslint\lib\rules\no-trailing-spaces.js
  130:21  error  This assigned value is not used in subsequent statements  no-useless-assignment

C:\projects\tmp\ota-meshi-eslint\lib\rules\utils\ast-utils.js
  1804:13  error  This assigned value is not used in subsequent statements  no-useless-assignment
  1805:13  error  This assigned value is not used in subsequent statements  no-useless-assignment

C:\projects\tmp\ota-meshi-eslint\lib\rules\yield-star-spacing.js
  79:21  error  This assigned value is not used in subsequent statements  no-useless-assignment

C:\projects\tmp\ota-meshi-eslint\lib\source-code\token-store\index.js
  40:9  error  This assigned value is not used in subsequent statements  no-useless-assignment
  41:9  error  This assigned value is not used in subsequent statements  no-useless-assignment

C:\projects\tmp\ota-meshi-eslint\tests\lib\cli-engine\cli-engine.js
  2510:21  error  This assigned value is not used in subsequent statements  no-useless-assignment
  2565:21  error  This assigned value is not used in subsequent statements  no-useless-assignment
  2622:21  error  This assigned value is not used in subsequent statements  no-useless-assignment

C:\projects\tmp\ota-meshi-eslint\tests\lib\eslint\eslint.js
  2464:21  error  This assigned value is not used in subsequent statements  no-useless-assignment
  2523:21  error  This assigned value is not used in subsequent statements  no-useless-assignment
  2583:21  error  This assigned value is not used in subsequent statements  no-useless-assignment

C:\projects\tmp\ota-meshi-eslint\tests\lib\eslint\flat-eslint.js
  2400:21  error  This assigned value is not used in subsequent statements  no-useless-assignment
  2465:21  error  This assigned value is not used in subsequent statements  no-useless-assignment
  2525:21  error  This assigned value is not used in subsequent statements  no-useless-assignment

C:\projects\tmp\ota-meshi-eslint\tests\lib\rule-tester\no-test-runners.js
  13:1  error  This assigned value is not used in subsequent statements  no-useless-assignment
  14:1  error  This assigned value is not used in subsequent statements  no-useless-assignment
  30:5  error  This assigned value is not used in subsequent statements  no-useless-assignment
  31:5  error  This assigned value is not used in subsequent statements  no-useless-assignment

C:\projects\tmp\ota-meshi-eslint\tools\update-readme.js
  152:9  error  This assigned value is not used in subsequent statements  no-useless-assignment

Most are initial assignments in declarations to default values like null, 0 or "". Perhaps these should be allowed in declarations (maybe optionally)?

There are also some that look to me like false positives for this rule, where the value is used on the right side of a subsequent assignment. For example:

eslint/Makefile.js

Lines 375 to 377 in 994596b

    
           let commits = execSilent(`git rev-list HEAD -- ${filePath}`); 
        
           commits = splitCommandResultToLines(commits);

mdjermanovic · 2023-10-23T13:11:51Z

This is another kind of false positive, I believe:

eslint/lib/linter/config-comment-parser.js

Lines 102 to 128 in 994596b

    
           items = {}; 
        
           const normalizedString = string.replace(/([-a-zA-Z0-9/]+):/gu, "\"$1\":").replace(/(\]|[0-9])\s+(?=")/u, "$1,"); 
        
           try { 
        
               items = JSON.parse(`{${normalizedString}}`); 
        
           } catch (ex) { 
        
               debug("Manual parsing failed."); 
        
               return { 
        
                   success: false, 
        
                   error: { 
        
                       ruleId: null, 
        
                       fatal: true, 
        
                       severity: 2, 
        
                       message: `Failed to parse JSON from '${normalizedString}': ${ex.message}`, 
        
                       line: location.start.line, 
        
                       column: location.start.column + 1, 
        
                       nodeType: null 
        
                   } 
        
               }; 
        
           } 
        
           return { 
        
               success: true, 
        
               config: items 
        
           };

The error is reported on line 102, but if JSON.parse() throws, that {} will be used as return value on line 127.

Edit: actually, this looks correct since catch always returns? Conclusion: this is not a false positive, the rule works fine in this case.

nzakas · 2023-12-11T15:59:39Z

tests/lib/rules/no-useless-assignment.js

Can you switch this to use FlatRuleTester instead? We are going through and updating all rule tests to do so. It should look like this:

const RuleTester = require("../../../lib/rule-tester/flat-rule-tester");

I changed the test case to use FlatRuleTester 👍

nzakas · 2023-12-14T16:37:22Z

@mdjermanovic this is ready for you to check on your suggestions.

lib/rules/no-useless-assignment.js

Co-authored-by: Milos Djermanovic <milos.djermanovic@gmail.com>

ota-meshi · 2023-12-17T12:32:48Z

Thank you for your suggestions and also for finding the false positives. I fixed them.

lib/rules/no-useless-assignment.js

Co-authored-by: Nicholas C. Zakas <nicholas@humanwhocodes.com>

nzakas

LGTM. Leaving open for @mdjermanovic to review.

mdjermanovic · 2024-01-09T08:12:00Z

LGTM, I'll just close-reopen to check CI before merging.

mdjermanovic

Great work, thanks!

Given the complexity of the problem and JS syntax, I wouldn't be surprised if some edge cases pop up in production, but can't see any at the moment.

Sorry for the delay in reviewing, there have been many tasks for the v9 release lately.

* main: chore: Split Docs CI from core CI (eslint#17897) fix: Ensure config keys are printed for config errors (eslint#17980) chore: delete relative-module-resolver.js (eslint#17981) docs: migration guide entry for `no-inner-declarations` (eslint#17977) docs: Update README feat: maintain latest ecma version in ESLint (eslint#17958) feat!: no-inner-declaration new default behaviour and option (eslint#17885) feat: add `no-useless-assignment` rule (eslint#17625) fix: `no-misleading-character-class` edge cases with granular errors (eslint#17970) feat: `no-misleading-character-class` granular errors (eslint#17515) docs: fix number of code-path events on custom rules page (eslint#17969) docs: reorder entries in v9 migration guide (eslint#17967) fix!: handle `--output-file` for empty output when saving to disk (eslint#17957)

feat: add no-useless-assignment rule

fa14942

eslint-github-bot bot added the feature This change adds a new feature to ESLint label Oct 7, 2023

ota-meshi added 2 commits October 7, 2023 16:14

fix: add test cases and fix

020ee4f

chore: remove wrong comment

35327ba

ota-meshi commented Oct 7, 2023

View reviewed changes

lib/rules/no-useless-assignment.js Outdated Show resolved Hide resolved

fasttime added accepted There is consensus among the team that this change meets the criteria for inclusion rule Relates to ESLint's core rules labels Oct 7, 2023

test: add test and update

ca5c5c2

ota-meshi marked this pull request as ready for review October 9, 2023 05:43

ota-meshi requested a review from a team as a code owner October 9, 2023 05:43

mdjermanovic reviewed Oct 9, 2023

View reviewed changes

tests/lib/rules/no-useless-assignment.js Show resolved Hide resolved

ota-meshi marked this pull request as draft October 9, 2023 12:49

ota-meshi added 2 commits October 9, 2023 23:36

feat: report if it is not used between assignments

3d63f0f

fix: add test case and fix

7e2692e

nzakas reviewed Oct 9, 2023

View reviewed changes

lib/rules/no-useless-assignment.js Outdated Show resolved Hide resolved

lib/rules/no-useless-assignment.js Outdated Show resolved Hide resolved

ota-meshi and others added 5 commits October 10, 2023 19:25

Apply suggestions from code review

6bb7f65

Co-authored-by: Nicholas C. Zakas <nicholas@humanwhocodes.com>

docs: update

fb050c3

fix: false negatives for same segments & refactor

eb5e982

Merge remote-tracking branch 'upstream/main' into no-useless-assignment

d9e3769

fix: rule-types.json

c8ab128

ota-meshi marked this pull request as ready for review October 20, 2023 01:21

mdjermanovic reviewed Oct 23, 2023

View reviewed changes

ota-meshi added 2 commits October 24, 2023 08:12

fix: add update assignment test case & fix

d3c1223

fix: add test for global vars & fix

96dd2ac

ota-meshi force-pushed the no-useless-assignment branch from 204860a to 96dd2ac Compare October 23, 2023 23:38

nzakas reviewed Dec 11, 2023

View reviewed changes

ota-meshi added 2 commits December 12, 2023 09:03

Merge remote-tracking branch 'upstream/main' into no-useless-assignment

d0e6ef9

test: use flat-rule-tester

6fce049

mdjermanovic reviewed Dec 15, 2023

View reviewed changes

ota-meshi and others added 5 commits December 17, 2023 21:22

Update lib/rules/no-useless-assignment.js

a83dfde

Co-authored-by: Milos Djermanovic <milos.djermanovic@gmail.com>

fix: false positives for finally block

8a10c7f

fix: false positives for markVariableAsUsed()

a6e5843

chore: rename function

cf5dd30

fix: false positives for unreachable segments

b3d0d92

ota-meshi requested a review from mdjermanovic December 21, 2023 14:10

nzakas reviewed Dec 22, 2023

View reviewed changes

lib/rules/no-useless-assignment.js Outdated Show resolved Hide resolved

ota-meshi and others added 3 commits December 27, 2023 15:44

Update lib/rules/no-useless-assignment.js

8ecc87c

Co-authored-by: Nicholas C. Zakas <nicholas@humanwhocodes.com>

Merge remote-tracking branch 'upstream/main' into no-useless-assignment

011330a

test: revert tests/conf/eslint-all.js

adf0fb4

nzakas approved these changes Jan 2, 2024

View reviewed changes

mdjermanovic closed this Jan 9, 2024

mdjermanovic reopened this Jan 9, 2024

ota-meshi added 2 commits January 9, 2024 17:14

Merge remote-tracking branch 'upstream/main' into no-useless-assignment

fe8053b

test: use rule-tester

f616420

mdjermanovic approved these changes Jan 9, 2024

View reviewed changes

mdjermanovic merged commit b4e0503 into eslint:main Jan 9, 2024
15 checks passed

ota-meshi deleted the no-useless-assignment branch January 9, 2024 08:48

rwlxxvii mentioned this pull request May 17, 2024

[Snyk] Upgrade eslint from 9.0.0 to 9.1.1 rwlxxvii/containers#114

Open

baby636 mentioned this pull request May 21, 2024

[Snyk] Upgrade eslint from 5.6.1 to 9.1.1 baby636/bitdb-testnet#14

Open

JoKacar mentioned this pull request May 25, 2024

[Snyk] Upgrade eslint from 8.57.0 to 9.2.0 JoKacar/codecov-action#6

Open

merfu mentioned this pull request May 25, 2024

[Snyk] Upgrade eslint from 5.16.0 to 9.2.0 merfu/aws-serverless-workshops#5

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: add `no-useless-assignment` rule #17625

feat: add `no-useless-assignment` rule #17625

ota-meshi commented Oct 7, 2023 •

edited

netlify bot commented Oct 7, 2023 •

edited

ota-meshi commented Oct 20, 2023

mdjermanovic commented Oct 20, 2023

mdjermanovic Oct 23, 2023

ota-meshi Oct 23, 2023

nzakas Oct 24, 2023

mdjermanovic commented Oct 23, 2023

mdjermanovic commented Oct 23, 2023 •

edited

nzakas Dec 11, 2023

ota-meshi Dec 12, 2023

nzakas commented Dec 14, 2023

ota-meshi commented Dec 17, 2023

nzakas left a comment

mdjermanovic commented Jan 9, 2024

mdjermanovic left a comment

feat: add no-useless-assignment rule #17625

feat: add no-useless-assignment rule #17625

Conversation

ota-meshi commented Oct 7, 2023 • edited

Prerequisites checklist

What is the purpose of this pull request? (put an "X" next to an item)

What changes did you make? (Give an overview)

Is there anything you'd like reviewers to focus on?

netlify bot commented Oct 7, 2023 • edited

✅ Deploy Preview for docs-eslint ready!

ota-meshi commented Oct 20, 2023

mdjermanovic commented Oct 20, 2023

mdjermanovic Oct 23, 2023

Choose a reason for hiding this comment

ota-meshi Oct 23, 2023

Choose a reason for hiding this comment

nzakas Oct 24, 2023

Choose a reason for hiding this comment

mdjermanovic commented Oct 23, 2023

mdjermanovic commented Oct 23, 2023 • edited

nzakas Dec 11, 2023

Choose a reason for hiding this comment

ota-meshi Dec 12, 2023

Choose a reason for hiding this comment

nzakas commented Dec 14, 2023

ota-meshi commented Dec 17, 2023

nzakas left a comment

Choose a reason for hiding this comment

mdjermanovic commented Jan 9, 2024

mdjermanovic left a comment

Choose a reason for hiding this comment

feat: add `no-useless-assignment` rule #17625

feat: add `no-useless-assignment` rule #17625

ota-meshi commented Oct 7, 2023 •

edited

netlify bot commented Oct 7, 2023 •

edited

mdjermanovic commented Oct 23, 2023 •

edited