Comments (15)
@ljharb At the moment, I'm inclined to think @nosilleg's approach to get the configuration is what we need.
Now bear in mind I'm not familiar with the internals of eslint and I'm not familiar at all with eslint-find-rules
. (The reason I'm here is that eslint-find-rules
's incompatibility with eslint 6 blocks the release of an eslint-6-compatible airbnb eslint configuration, which my own stock eslint configuration depends on. So I'm blocked from moving to eslint 6 due to the issue here. I figured I could whine or try to help, and I decided to try to help rather than whine.)
It does appear that in eslint 6, cliEngine.getConfigForFile("./does-not-exist.a9<");
does the same as cliEngine.getConfigForFile()
did in eslint 5. So this preserves the status quo.
This essentially prevents the overrides
sections of configuration files from applying but:
-
eslint-find-rules
was also ignoring them when callinggetConfigForFile
without parameters. -
eslint supports cascading configurations, which
eslint-find-rules
has also been ignoring by callinggetConfigForFile
without parameters.
Now maybe there's an argument to be made that eslint-find-rules
ignoring overrides
and cascading configurations was wrong and now it should take both into account. If indeed eslint-find-rules
should be modified to take into account overrides
and cascading configurations, I have three observations:
-
This would be a breaking change: there exist some configurations and file trees for which the results returned by
eslint-find-rules
would change. -
I'd suggest still allowing a flag (e.g. an
--only-invariant
flag on the command line) that would allow those users who only need the old behavior to still get this behavior and not scan the disk for files needlessly. -
eslint-find-rules
should allow users to specify the pattern of files to search rather than use a hardcoded search. For instance, although these use the sameconfig.js
file, this command:eslint -c config.js 'src/**/*.js'
can use a different set of rules from this one:
eslint -c config.js 'test/**/*.js'
I'd expect
eslint-find-rules
to take a file pattern likeeslint
does. Otherwise, the configuration used byeslint-find-rules
will correspond to a real run ofeslint
only sometimes, accidentally.(Also, eslint supports TypeScript, and the plan is to eventually have it replace
tslint
as the go-to linter for TypeScript code. (See this announcement.) Searching for**/*.js
won't work for some users of eslint. They'd want to search for**/*.ts
.)
from eslint-find-rules.
@ljharb I took your branch and added a commit to deal with the test failures. My branch is here. The commit I added is this one.
The problem was that eslint 6 tries to resolve plugin module names to file names prior to calling require
and that's something that proxyquire
does not handle automatically.
With my commit, all tests pass.
from eslint-find-rules.
My first attempt is here: master...ljharb:eslint_6
from eslint-find-rules.
@ljharb Glancing quickly at the CI report, shouldn't there be also be a bunch of tests with ESLINT=6
?
I see two different failures:
-
Can't load
eslint/lib/shared/relative-module-resolver
. That's going to happen on eslint < 6 since that file does not exist there. Atry... catch
around the offendingrequire
allows tests to pass on eslint 5. I've amended my commit to take care of that. -
The other failure I'm seeing is are linting errors which appear to be due to the changes you made in your commit. When I sent my comment yesterday I had noticed this linting failure but I was very pressed for time and concerned more about providing a heads up to make sure somebody's time was not wasted duplicating my work than about any other consideration. I've added a commit that fixes this issue.
I've force-pushed my branch just now.
from eslint-find-rules.
I've had a go at trying to do this and it seems the major issue is that getConfigForFile
that is used here - https://github.com/sarbbottam/eslint-find-rules/blob/5c192eb/src/lib/rule-finder.js#L26 - now requires a file path to be passed in. I think in previous ESLint versions it also wanted one but would fall back to cwd
. I think it now requires a file because depending on the file path passed in a different set of rules could be present (through the use of overrides
maybe?).
So there are some decisions to be made about how to get around this.
from eslint-find-rules.
cc @mysticatea @platinumazure who i think were involved with this eslint change
from eslint-find-rules.
Per our document, --print-config
and getConfigForFile()
require a file path. That fallback was not intentional.
ESLint may use different configs for each source code file because it supports cascading and glob-based configuration. Because I'm not familiar with this tool, I'm not sure if what decision is proper.
from eslint-find-rules.
@mysticatea ideally it would take a directory or a glob; it could even be an array of all found config files within a directory.
The intention of the tool is to help manage config files themselves.
from eslint-find-rules.
Please mind getConfigForFile()
is the method to get the config for a file, so the name is getConfigForFile()
. :)
A config file doesn't 1:1 map to a config object that getConfigForFile()
method returns because the method determines which overrides
entries were applied. The criteria of overrides
entries can overlap each other. This means that it will have to return all combinations of overrides
entries in all of the config files and shareable configs. We can propose such an API but I'm worried about a combinatorial explosion.
For now, how about...
const glob = require("glob")
const { CLIEngine } = require("eslint")
const engine = new CLIEngine()
function getConfigs(pattern) {
const configs = new Set()
for (const filePath of glob.sync(pattern, { dot: true, matchBase: true })) {
if (!engine.isPathIgnored(filePath)) {
configs.add(engine.getConfigForFile(filePath))
}
}
return configs
}
console.log(getConfigs("lib/*.js"))
This should work both of ESLint 6 and older versions.
from eslint-find-rules.
This approach definitely seems to work to get the configured rules; unfortunately it doesn't seem to be sufficient to make this package work on eslint 6 :-/
from eslint-find-rules.
@ljharb Are there any issues with your first attempt that are preventing a merge? (I'm not able to run the code given my current location/circumstances).
My initial thought from looking at the code is that it can report different rules/plugins than the current version since your new code will account for some overrides. The fact that the overrides can have non *.js
(as per the new glob) criteria means that not all overrides will be evaluated.
To ovoid those differences would replacing the current getConfigForFile()
with getConfigForFile("./does-not-exist.a9<");
suffice?
This could match an overrides
, but should be less likely.
from eslint-find-rules.
@nosilleg it’s not complete, so it doesn’t work yet. If you have any suggestions and want to make a PR, that would be appreciated :-)
from eslint-find-rules.
@ljharb I'm not going to be able to run code for at least a week, so can't do a full PR and test run.
Based on the discussions and a read of the code the proposed change would be to simply change this line: https://github.com/sarbbottam/eslint-find-rules/blob/master/src/lib/rule-finder.js#L26
To:
return cliEngine.getConfigForFile("./does-not-exist.a9<");
A quick test on RunKit seems to give useful results. https://runkit.com/5d4493461549b300131329cf/5d4493541549b300131329e1
Obviously since I haven't run the code there may be things that I'm missing
from eslint-find-rules.
@lddubeau thanks, that's super appreciated!
I'm still getting some failures locally; I'm seeing different failures in CI: https://travis-ci.com/ljharb/eslint-find-rules/builds/121896205
from eslint-find-rules.
That's a pretty compelling argument for that approach. Let me pull in your commit.
from eslint-find-rules.
Related Issues (20)
- Release new version with support eslint@5 HOT 1
- print as list HOT 2
- Scoped plugin error HOT 4
- Security warning for "mem" HOT 12
- Doesn't find unused rules in @typescript-eslint/eslint-plugin HOT 4
- Support overrides HOT 14
- eslint v7 support
- Handle typescript-eslint configuration HOT 1
- git clone error on windows HOT 6
- I'm not sure how to deal with renamed rule in ESLint 7.4.0 HOT 7
- Find unused plugin rules
- CHANGELOG missing HOT 3
- Breaks with ESLint 7.8.0 HOT 4
- Update dependencies HOT 5
- Support ESLint 8.x HOT 2
- Add support to the `rule.meta.docs.url` property to get URLs in verbose mode
- Rules in the "rules" json key are not being considered HOT 1
- No Option Provided with `--option` described in docs HOT 1
- Not able to save output as single column HOT 1
Recommend Projects
-
React
A declarative, efficient, and flexible JavaScript library for building user interfaces.
-
Vue.js
🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
-
Typescript
TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
-
TensorFlow
An Open Source Machine Learning Framework for Everyone
-
Django
The Web framework for perfectionists with deadlines.
-
Laravel
A PHP framework for web artisans
-
D3
Bring data to life with SVG, Canvas and HTML. 📊📈🎉
-
Recommend Topics
-
javascript
JavaScript (JS) is a lightweight interpreted programming language with first-class functions.
-
web
Some thing interesting about web. New door for the world.
-
server
A server is a program made to process requests and deliver data to clients.
-
Machine learning
Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.
-
Visualization
Some thing interesting about visualization, use data art
-
Game
Some thing interesting about game, make everyone happy.
Recommend Org
-
Facebook
We are working to build community through open source technology. NB: members must have two-factor auth.
-
Microsoft
Open source projects and samples from Microsoft.
-
Google
Google ❤️ Open Source for everyone.
-
Alibaba
Alibaba Open Source for everyone
-
D3
Data-Driven Documents codes.
-
Tencent
China tencent open source team.
from eslint-find-rules.