search

nickdeis / eslint-plugin-no-secrets

An eslint plugin to find strings that might be secrets/credentials
MIT License
127 stars 4 forks source link

[WIP] Remove JSON preprocessor to leave this job to another plugins #13

Closed constgen closed 3 years ago

constgen commented 3 years ago

Have an issue when use this plugin together with https://github.com/ota-meshi/eslint-plugin-jsonc . The problem is that this plugin defines its own processor for JSON files when it shouldn't. We need to remove this and leave preprocessing to dedicated plugins. eslint-plugin-jsonc particularly will inherit all rules applied to JS tokens to JSON tokens which means that eslint-plugin-no-secrets will naturally work in JSON. And also it will work in JSONC, JSON5 and JSON6 which this plugin doesn't support

The error thrown during linting JSON files only:

error: Parsing error: Unexpected token var at src\configs\vue.config.json:1:3:
nickdeis commented 3 years ago

Hey @constgen, Thanks for taking the time to create a PR. A huge part of me agrees. I've already ran into this issue, and have filed an issue on the eslint codebase about it (#11). The problem is that sometimes (and some other plugins that create processors) don't process things in a way that can be used by another plugin. I will have to analyze the https://github.com/ota-meshi/eslint-plugin-jsonc codebase and see if it does return a useful processed output. If it does, I will accept this PR and add additional documentation to allow users to still have JSON supported with my plugin. I will get back to you soon with what I think, and then I will probably add some unit tests/integration tests. Best, Nick

nickdeis commented 3 years ago

Hey @constgen, Do you have any idea how they parse the jsons? They aren't using processor, so I have no idea what the signature would be. Best, Nick

constgen commented 3 years ago

Most likely they use parsers. But some of them use preprocessors. ESLint doesn't merge or pipe preprocessors. Plugins just override preprocessors of each other. Implementation may be really different. For example, eslint-plugin-jsonc uses its own parser jsonc-eslint-parser that outputs different AST tokens. Unfortunately it doesn't share any tokens from JavaScript (e.g. for strings). This means I was not right when said "eslint-plugin-no-secrets will naturally work in JSON"

You can see the AST output in this demo https://ota-meshi.github.io/jsonc-eslint-parser/

What additional options I see:

  1. We can integrate with AST tokens of jsonc-eslint-parser specifically
  2. Talk with jsonc-eslint-parser to produce common JavaScript AST nodes to inherit all rules for strings, numbers, etc. Then this plugin will not have to think about the JSON syntax. It will just work
nickdeis commented 3 years ago

Hey @constgen , Sorry about the delay, I've had some family issues recently. This is a really elegant solution. I have implemented it in PR #14. Please let me know what you think and if this solution is what you had in mind. I will still merge this PR since it represents the first step. Thank you for all the research and useful information you have provided, this has been a real issue with a lot of my users. Best, Nick

nickdeis commented 3 years ago

Implemented by PR #14

nickdeis commented 3 years ago

Going to close this PR. I want to thank you @constgen for all the help, as this problem has been bothering me since June of last year.