migrating import/default rule to parsers.all function #2482
Conversation
Codecov Report
@@ Coverage Diff @@
## parsers #2482 +/- ##
========================================
Coverage 95.03% 95.03%
========================================
Files 66 66
Lines 2799 2799
Branches 940 940
========================================
Hits 2660 2660
Misses 139 139 Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. |
tests/helpers/getESLintCoreRule.js
Outdated
| // eslint-disable-next-line global-require, import/no-dynamic-require, import/no-unresolved | ||
| const getESLintCoreRule = (ruleId) => (isESLintV8 ? require('eslint/use-at-your-own-risk').builtinRules.get(ruleId) : require(`eslint/lib/rules/${ruleId}`)); | ||
|
|
||
| module.exports = getESLintCoreRule; |
There was a problem hiding this comment.
It was an oversight. It wasn't meant to be pushed
|
(I changed the base of this so it's into a "parsers" branch) |
|
Am I going in the right direction ? @ljharb |
|
Pulling in the parsers.all helper is great :-) but the rule tests haven't been touched yet, and that'd be the important part. |
tests/src/rules/default.js
Outdated
| import { RuleTester } from 'eslint'; | ||
| import semver from 'semver'; | ||
| import { version as tsEslintVersion } from 'typescript-eslint-parser/package.json'; | ||
|
|
||
| import { CASE_SENSITIVE_FS } from 'eslint-module-utils/resolve'; | ||
|
|
||
| const ruleTester = new RuleTester(); | ||
| const RuleTester = require('eslint').RuleTester; |
There was a problem hiding this comment.
| const RuleTester = require('eslint').RuleTester; |
this is already imported
| ruleTester.run('default', rule, { | ||
| valid: [].concat( | ||
| test({ code: 'import "./malformed.js"' }), |
There was a problem hiding this comment.
do we no longer need the test function? that function is highly specific to the import plugin, so i wouldn't want its logic to live inside parsers.all
There was a problem hiding this comment.
Oh okay. I'll add them back
There was a problem hiding this comment.
We could also do .map(x => test(x)) on the array, if they don't pass any options to test - up to you. might be better to minimize the diff here.
tests/src/rules/default.js
Outdated
| // #311: import of mismatched case | ||
| if (!CASE_SENSITIVE_FS) { |
There was a problem hiding this comment.
it would also be ideal to combine this so that the entire file has a single list of "valid" and "invalid" cases, rather than multiples.
There was a problem hiding this comment.
Would this also include the context function. It has valid and invalid cases too
ljharb
force-pushed
the
parsers
branch
2 times, most recently
from
fc4875f
to
5cff58e
Compare
Co-authored-by: Lubwama Collins <bizzicole87@gmail.com> Co-authored-by: Jordan Harband <ljharb@gmail.com>
Co-authored-by: Lubwama Collins <bizzicole87@gmail.com> Co-authored-by: Jordan Harband <ljharb@gmail.com>
ljharb
force-pushed
the
parsers
branch
2 times, most recently
from
e81ddfd
to
dd74311
Compare
ljharb
force-pushed
the
default-rule
branch
2 times, most recently
from
e2d8e6a
to
efa7e9d
Compare
|
This is looking great! We're down to a manageable number of failures :-) |
Migration of import/default rule to parsers.all function
Closes #2498.