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

prevent-abbreviations: Fix objects must not be overlapped in a report. #912

Closed
AriPerkkio opened this issue Dec 3, 2020 · 12 comments · Fixed by #913
Closed

prevent-abbreviations: Fix objects must not be overlapped in a report. #912

AriPerkkio opened this issue Dec 3, 2020 · 12 comments · Fixed by #913
Assignees
Labels

Comments

@AriPerkkio
Copy link
Contributor

Hello, I'm testing stability of well known community ESLint plugins with eslint-remote-tester. This ESLint plugin seems to contain a rule which causes linter to crash. ESLint rules should not crash in any condition since this makes all valid linting problems disappear. If this is a false flag please let me know.

This issue was spotted by automated CI run: https://github.com/AriPerkkio/eslint-remote-tester/runs/1487452403

Crashing rule:

prevent-abbreviations

If name of the VariableExpression and name of the TSTypeReference are identical unicorn/prevent-abbreviations crashes.

Minimal repro:

This seems to be valid typescript:
https://www.typescriptlang.org/play?#code/JYOwLgpgTgZghgYwgAgApQPYAdkG8BQyRywAJgFzIgCuAtgEbQDc+AvvvghiAM5hqYsldNmQBePCQrIAjMlYt8EAB5YMUfqQjxqAG34isTIA

interface Prop {
    id: number;
}

const Prop: Prop = { id: 1 };

export default Prop;

This causes ESlint to dispaly an error:

$ ./node_modules/.bin/eslint lib

Oops! Something went wrong! :(

ESLint: 7.12.1

AssertionError [ERR_ASSERTION]: Fix objects must not be overlapped in a report.
    at mergeFixes (/<root-path-removed>/node_modules/eslint/lib/linter/report-translator.js:152:9)
    at normalizeFixes (/<root-path-removed>/node_modules/eslint/lib/linter/report-translator.js:182:16)
    at /<root-path-removed>/node_modules/eslint/lib/linter/report-translator.js:347:49
    at Object.report (/<root-path-removed>/node_modules/eslint/lib/linter/linter.js:920:41)
    at checkVariable (/<root-path-removed>/node_modules/eslint-plugin-unicorn/rules/prevent-abbreviations.js:653:11)
    at checkPossiblyWeirdClassVariable (/<root-path-removed>/node_modules/eslint-plugin-unicorn/rules/prevent-abbreviations.js:567:10)
    at /<root-path-removed>/node_modules/eslint-plugin-unicorn/rules/prevent-abbreviations.js:657:39
    at Array.forEach (<anonymous>)
    at checkVariables (/<root-path-removed>/node_modules/eslint-plugin-unicorn/rules/prevent-abbreviations.js:657:19)
    at checkScope (/<root-path-removed>/node_modules/eslint-plugin-unicorn/rules/prevent-abbreviations.js:665:3)
Errors from real-world projects

Rule: prevent-abbreviations

  • Message: Fix objects must not be overlapped in a report. Occurred while linting <text>:1
  • Path: microsoft/fluentui/packages/codemods/src/modRunner/tests/mocks/MockMods/CodeMod.mock.ts
  • Link
AssertionError [ERR_ASSERTION]: Fix objects must not be overlapped in a report.
    at mergeFixes (/<root-path-removed>/node_modules/eslint/lib/linter/report-translator.js:152:9)
    at normalizeFixes (/<root-path-removed>/node_modules/eslint/lib/linter/report-translator.js:182:16)
    at /<root-path-removed>/node_modules/eslint/lib/linter/report-translator.js:347:49
    at Object.report (/<root-path-removed>/node_modules/eslint/lib/linter/linter.js:920:41)
    at checkVariable (/<root-path-removed>/ci/node_modules/eslint-plugin-unicorn/rules/prevent-abbreviations.js:653:11)
    at checkPossiblyWeirdClassVariable (/<root-path-removed>/ci/node_modules/eslint-plugin-unicorn/rules/prevent-abbreviations.js:567:10)
    at /<root-path-removed>/ci/node_modules/eslint-plugin-unicorn/rules/prevent-abbreviations.js:657:39
    at Array.forEach (<anonymous>)
    at checkVariables (/<root-path-removed>/ci/node_modules/eslint-plugin-unicorn/rules/prevent-abbreviations.js:657:19)
    at checkScope (/<root-path-removed>/ci/node_modules/eslint-plugin-unicorn/rules/prevent-abbreviations.js:665:3)

Rule: prevent-abbreviations

  • Message: Fix objects must not be overlapped in a report. Occurred while linting <text>:1
  • Path: microsoft/fluentui/packages/fluentui/docs/src/components/ComponentDoc/ComponentPropsTable/ComponentPropsTable.tsx
  • Link
AssertionError [ERR_ASSERTION]: Fix objects must not be overlapped in a report.
    at mergeFixes (/<root-path-removed>/node_modules/eslint/lib/linter/report-translator.js:152:9)
    at normalizeFixes (/<root-path-removed>/node_modules/eslint/lib/linter/report-translator.js:182:16)
    at /<root-path-removed>/node_modules/eslint/lib/linter/report-translator.js:347:49
    at Object.report (/<root-path-removed>/node_modules/eslint/lib/linter/linter.js:920:41)
    at checkVariable (/<root-path-removed>/ci/node_modules/eslint-plugin-unicorn/rules/prevent-abbreviations.js:653:11)
    at checkPossiblyWeirdClassVariable (/<root-path-removed>/ci/node_modules/eslint-plugin-unicorn/rules/prevent-abbreviations.js:567:10)
    at /<root-path-removed>/ci/node_modules/eslint-plugin-unicorn/rules/prevent-abbreviations.js:657:39
    at Array.forEach (<anonymous>)
    at checkVariables (/<root-path-removed>/ci/node_modules/eslint-plugin-unicorn/rules/prevent-abbreviations.js:657:19)
    at checkScope (/<root-path-removed>/ci/node_modules/eslint-plugin-unicorn/rules/prevent-abbreviations.js:665:3)
@AriPerkkio
Copy link
Contributor Author

@sindresorhus if you are interested these crashing rules could be identified by scheduled smoke tests. @testing-library/eslint-plugin-jest-dom is already using similar approach: testing-library/eslint-plugin-jest-dom#108. They've started to spot bugs earlier now.

I can setup PR if this seems useful.

@fisker
Copy link
Collaborator

fisker commented Dec 4, 2020

I can setup PR if this seems useful.

Is it similar to our integration test?

@fisker
Copy link
Collaborator

fisker commented Dec 4, 2020

I'm going to add microsoft/fluentui to see if we can caught this error.

@AriPerkkio
Copy link
Contributor Author

Is it similar to our integration test?

Looks quite similar. It clones given repositories and runs ESLint on files of the repository. Files are also filtered based on file size, file exclusion pattern and file extension. Tests are run on multiple threads via worker_threads. You can see all configuration options from the project repository.

I've included this ESLint plugin to my scheduled CI runs. These are ran against 150 repositories, but for a single ESLint plugin it can easily be run against 500-1000 repositories for fairly short time. eslint-plugin-jest-dom is using ~500 repos and their runs take ~30 mins.

So far these scheduled runs have spotted three issues from this plugin. You might want to add their repositories in your tests as well.

@fisker
Copy link
Collaborator

fisker commented Dec 4, 2020

Have you run this on https://github.com/angular/angular, how long will it take to finish?

@AriPerkkio
Copy link
Contributor Author

https://github.com/AriPerkkio/eslint-remote-tester/actions/runs/400370452

2020-12-04T09:44:59.9814689Z [STARTING] angular/angular
2020-12-04T09:45:00.8470164Z [CLONING] angular/angular
2020-12-04T09:45:07.1292427Z [READING] angular/angular
2020-12-04T09:45:07.5325792Z [LINTING] angular/angular - 7235 files
2020-12-04T09:47:18.1100434Z [DONE] angular/angular 0 errors
2020-12-04T09:47:18.1101915Z [DONE] Finished scan of 1 repositories
2020-12-04T09:47:18.1450667Z 
2020-12-04T09:47:18.1452038Z Results:
2020-12-04T09:47:18.1453248Z No errors
2020-12-04T09:47:18.1526302Z 
2020-12-04T09:47:18.1604167Z Done in 139.51s.

@fisker
Copy link
Collaborator

fisker commented Dec 4, 2020

That's fast!

@sindresorhus
Copy link
Owner

This sounds great to me. Would be nice to be able to test even more repos.

@AriPerkkio Do you plan to actively maintain eslint-remote-tester?

@AriPerkkio
Copy link
Contributor Author

AriPerkkio commented Dec 6, 2020

Would be nice to be able to test even more repos

I've ran eslint-plugin-unicorn against 1832 repositories. The CI run takes ~2 hours. I can use these for the initial PR. Here is one example CI run: https://github.com/AriPerkkio/eslint-plugin-unicorn/actions/runs/404115616

Do you plan to actively maintain eslint-remote-tester?

I sure do! I find this project very exciting. It's become very powerful tool for testing various ESLint plugins.

@sindresorhus
Copy link
Owner

We could maybe set it to run a limited number of repos for each PR/commit and also create a GitHub Actions cron job that runs once a week which runs a huge amount of repos. Then the run time doesn't matter.

@fisker
Copy link
Collaborator

fisker commented Dec 7, 2020

How about still run on these repos in this file for PRs.

@sindresorhus
Copy link
Owner

Yes, that's what I meant. But many more repos in addition once a week.

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.

3 participants