From 4af9338c3ec5f2355b2bc6c122c2bbe2e3b42150 Mon Sep 17 00:00:00 2001 From: Toru Nagashima Date: Tue, 16 Jul 2019 04:05:49 +0900 Subject: [PATCH 1/2] New: RuleTester supports processor --- designs/2019-rule-tester-processors/README.md | 70 +++++++++++++++++++ 1 file changed, 70 insertions(+) create mode 100644 designs/2019-rule-tester-processors/README.md diff --git a/designs/2019-rule-tester-processors/README.md b/designs/2019-rule-tester-processors/README.md new file mode 100644 index 00000000..46ba8252 --- /dev/null +++ b/designs/2019-rule-tester-processors/README.md @@ -0,0 +1,70 @@ +- Start Date: 2019-07-16 +- RFC PR: (leave this empty, to be filled in later) +- Authors: Toru Nagashima ([@mysticatea](https://github.com/mysticatea)) + +# RuleTester supports processor + +## Summary + +This RFC makes `RuleTester` class supporting `processor` option. + +## Motivation + +Currently, we cannot test rules with processors. This is inconvenient for plugin rules which depend on processors. For example, [vue/comment-directive](https://github.com/vuejs/eslint-plugin-vue/blob/6751ff47b4ecd722bc2e2436ce6b34a510f92b07/tests/lib/rules/comment-directive.js) rule could not use `RuleTester` to test. + +## Detailed Design + +To add `processor` option and `processorOptions` (see #29) to test cases. + +```js +const { RuleTester } = require("eslint") +const rule = require("../../../lib/rules/example-rule") +const exampleProcessor = require("../../../lib/processors/example-processor") + +const tester = new RuleTester() + +tester.run("example-rule", rule, { + valid: [ + { + code: ` + + `, + processor: exampleProcessor, + processorOptions: {}, + }, + ], + invalid: [], +}) +``` + +### § `processor` option + +This is a definition object of a processor that the "[Processors in Plugins](https://eslint.org/docs/developer-guide/working-with-plugins#processors-in-plugins)" section describes. + +If this option was given, the tester applies the given processor to test code. + +If the given processor didn't has `supportsAutofix:true`, the tester doesn't do autofix. Then if the test case had `output` option (except `null`) then the test case will fail. + +### § `processorOptions` option + +RFC #29 defines the `processorOptions`. + +If this option was given along with `processor` option, it will be given to the processor. + +## Documentation + +The [RuleTester](https://eslint.org/docs/developer-guide/nodejs-api#ruletester) section should describe the new `processor` and `processorOptions` properties. + +## Drawbacks + +It's a pretty rare case of a rule depends on the processor's behavior. I'm not sure if this feature is needed. + +## Backwards Compatibility Analysis + +There are no concerns about breaking changes. + +## Related Discussions + +- https://github.com/eslint/rfcs/pull/25#issuecomment-499877621 From 07fd2eb473e9cfff5079af9c257e0ce3e84849e9 Mon Sep 17 00:00:00 2001 From: Brandon Mills Date: Mon, 1 Nov 2021 01:45:59 -0400 Subject: [PATCH 2/2] Update RFC --- designs/2019-rule-tester-processors/README.md | 42 +++++++++++++++---- 1 file changed, 33 insertions(+), 9 deletions(-) diff --git a/designs/2019-rule-tester-processors/README.md b/designs/2019-rule-tester-processors/README.md index 46ba8252..274b7ad2 100644 --- a/designs/2019-rule-tester-processors/README.md +++ b/designs/2019-rule-tester-processors/README.md @@ -1,6 +1,6 @@ - Start Date: 2019-07-16 - RFC PR: (leave this empty, to be filled in later) -- Authors: Toru Nagashima ([@mysticatea](https://github.com/mysticatea)) +- Authors: Toru Nagashima ([@mysticatea](https://github.com/mysticatea)), Brandon Mills ([@btmills](https://github.com/btmills)) # RuleTester supports processor @@ -10,7 +10,7 @@ This RFC makes `RuleTester` class supporting `processor` option. ## Motivation -Currently, we cannot test rules with processors. This is inconvenient for plugin rules which depend on processors. For example, [vue/comment-directive](https://github.com/vuejs/eslint-plugin-vue/blob/6751ff47b4ecd722bc2e2436ce6b34a510f92b07/tests/lib/rules/comment-directive.js) rule could not use `RuleTester` to test. +Currently, we cannot test rules with processors. This is inconvenient for plugin rules which depend on processors. For example, [vue/comment-directive](https://github.com/vuejs/eslint-plugin-vue/blob/6751ff47b4ecd722bc2e2436ce6b34a510f92b07/tests/lib/rules/comment-directive.js) rule could not use `RuleTester` to test. Rules that distinguish between physical and virtual filenames [cannot be tested without processors](https://github.com/eslint/eslint/issues/14800). ## Detailed Design @@ -43,28 +43,52 @@ tester.run("example-rule", rule, { This is a definition object of a processor that the "[Processors in Plugins](https://eslint.org/docs/developer-guide/working-with-plugins#processors-in-plugins)" section describes. -If this option was given, the tester applies the given processor to test code. +If a processor is given, the tester passes its `preprocess`, `postprocess`, and optional `supportsAutofix` properties as part of `Linter#verify()`'s `filenameOrOptions` options object. -If the given processor didn't has `supportsAutofix:true`, the tester doesn't do autofix. Then if the test case had `output` option (except `null`) then the test case will fail. +The tester only applies fixes if the processor also has `supportsAutofix: true`. ### § `processorOptions` option -RFC #29 defines the `processorOptions`. +RFC #29 defines the `processorOptions`, though it has not yet been implemented. If this option was given along with `processor` option, it will be given to the processor. ## Documentation -The [RuleTester](https://eslint.org/docs/developer-guide/nodejs-api#ruletester) section should describe the new `processor` and `processorOptions` properties. +The [RuleTester](https://eslint.org/docs/developer-guide/nodejs-api#ruletester) section should describe the new `processor` and, if implemented, `processorOptions` properties. ## Drawbacks -It's a pretty rare case of a rule depends on the processor's behavior. I'm not sure if this feature is needed. +This expands `RuleTester`'s purpose to include testing processor-aware rules. +Some could see this as out of scope for `RuleTester`. + +It adds additional complexity to `RuleTester`, though it is minimal. + +This design only supports a single processor. ## Backwards Compatibility Analysis -There are no concerns about breaking changes. +`RuleTester` currently accepts the `processor` key as a string in valid and invalid test case objects, but it does nothing with that string. +It throws an error if `processor` is set to any non-string type. +If someone has set `processor` to a string value in any of their test cases, those tests would throw with this change. +Because the `processor` key does nothing prior to this change, having it is unlikely, and the fix is easy: deleting the `processor` key leaves the test logically unchanged. + +## Alternatives + +- To support testing rules that distinguish between physical and virtual filenames, we could instead add a `physicalFilename` option to test cases and modify `Linter` to use that option instead of computing filenames. + +## Open Questions + +- Does this design need to accept multiple nested processors? + +## Help Needed + +I can implement this myself. + +## Frequently Asked Questions +None yet. ## Related Discussions -- https://github.com/eslint/rfcs/pull/25#issuecomment-499877621 +- https://github.com/eslint/rfcs/pull/25#issuecomment-499877621 +- https://github.com/eslint/eslint/issues/14800