From 9ad5dbc869c53953d31ee152407e23129d0fd1fc Mon Sep 17 00:00:00 2001 From: Chris Meyer <32752981+EasyRhinoMSFT@users.noreply.github.com> Date: Wed, 16 Jan 2019 15:26:22 -0800 Subject: [PATCH 01/22] Create Readme.MD --- designs/2019-PassRulesToFormatters/Readme.MD | 1 + 1 file changed, 1 insertion(+) create mode 100644 designs/2019-PassRulesToFormatters/Readme.MD diff --git a/designs/2019-PassRulesToFormatters/Readme.MD b/designs/2019-PassRulesToFormatters/Readme.MD new file mode 100644 index 00000000..3b94f915 --- /dev/null +++ b/designs/2019-PassRulesToFormatters/Readme.MD @@ -0,0 +1 @@ +Placeholder From 9dbaa1d4a95dc96003205be04815175bd72a1cd2 Mon Sep 17 00:00:00 2001 From: Chris Meyer <32752981+EasyRhinoMSFT@users.noreply.github.com> Date: Wed, 16 Jan 2019 15:29:13 -0800 Subject: [PATCH 02/22] Delete Readme.MD --- designs/2019-PassRulesToFormatters/Readme.MD | 1 - 1 file changed, 1 deletion(-) delete mode 100644 designs/2019-PassRulesToFormatters/Readme.MD diff --git a/designs/2019-PassRulesToFormatters/Readme.MD b/designs/2019-PassRulesToFormatters/Readme.MD deleted file mode 100644 index 3b94f915..00000000 --- a/designs/2019-PassRulesToFormatters/Readme.MD +++ /dev/null @@ -1 +0,0 @@ -Placeholder From a1e73e7966cd6853d81bf6009e820f23c0fcdc7e Mon Sep 17 00:00:00 2001 From: Chris Meyer <32752981+EasyRhinoMSFT@users.noreply.github.com> Date: Wed, 16 Jan 2019 15:30:09 -0800 Subject: [PATCH 03/22] Create readme.MD --- designs/2019-expose-rules-to-formatters/readme.MD | 1 + 1 file changed, 1 insertion(+) create mode 100644 designs/2019-expose-rules-to-formatters/readme.MD diff --git a/designs/2019-expose-rules-to-formatters/readme.MD b/designs/2019-expose-rules-to-formatters/readme.MD new file mode 100644 index 00000000..3b94f915 --- /dev/null +++ b/designs/2019-expose-rules-to-formatters/readme.MD @@ -0,0 +1 @@ +Placeholder From 913614f64992280679643da3dcff2e806032c912 Mon Sep 17 00:00:00 2001 From: Chris Meyer <32752981+EasyRhinoMSFT@users.noreply.github.com> Date: Wed, 16 Jan 2019 16:10:56 -0800 Subject: [PATCH 04/22] Apply RFC template --- .../2019-expose-rules-to-formatters/readme.MD | 104 +++++++++++++++++- 1 file changed, 103 insertions(+), 1 deletion(-) diff --git a/designs/2019-expose-rules-to-formatters/readme.MD b/designs/2019-expose-rules-to-formatters/readme.MD index 3b94f915..069fc321 100644 --- a/designs/2019-expose-rules-to-formatters/readme.MD +++ b/designs/2019-expose-rules-to-formatters/readme.MD @@ -1 +1,103 @@ -Placeholder +- Start Date: (fill me in with today's date, YYYY-MM-DD) +- RFC PR: (leave this empty, to be filled in later) +- Authors: (the names of everyone contributing to this RFC) + +# (RFC title goes here) + +## Summary + + + +## Motivation + + + +## Detailed Design + + + +## Documentation + + + +## Drawbacks + + + +## Backwards Compatibility Analysis + + + +## Alternatives + + + +## Open Questions + + + +## Help Needed + + + +## Frequently Asked Questions + + + +## Related Discussions + + From 2edc789f1684c4e161f6586d8a088ad3b27a1eaa Mon Sep 17 00:00:00 2001 From: Chris Meyer <32752981+EasyRhinoMSFT@users.noreply.github.com> Date: Thu, 17 Jan 2019 12:42:37 -0800 Subject: [PATCH 05/22] Completed all but Alternatives and FAQ --- .../2019-expose-rules-to-formatters/readme.MD | 120 +++++++++++------- 1 file changed, 77 insertions(+), 43 deletions(-) diff --git a/designs/2019-expose-rules-to-formatters/readme.MD b/designs/2019-expose-rules-to-formatters/readme.MD index 069fc321..39652b31 100644 --- a/designs/2019-expose-rules-to-formatters/readme.MD +++ b/designs/2019-expose-rules-to-formatters/readme.MD @@ -1,56 +1,92 @@ -- Start Date: (fill me in with today's date, YYYY-MM-DD) +- Start Date: 2019-01-16 - RFC PR: (leave this empty, to be filled in later) -- Authors: (the names of everyone contributing to this RFC) +- Authors: Chris Meyer (@EasyRhinoMSFT) -# (RFC title goes here) +# Providing Rule Metadata to Formatters ## Summary - +This proposal describes a design enhancement that provides formatters with details about the rules that have been executed by ESLint. ## Motivation - +Currently, formatters only see the ID of each rule for which a violation was identified, plus an instance-specific description, as properties on each result object. Formatters are not able to access usefule rule metadata, such as category, description, and help URL. Formatters are also not aware of the full set of rules that were run, information that may be useful in some cases. ## Detailed Design - +### Command Line Interface (cli.js) Changes +The implementation of this feature is exceedingly simple, consisting of a straightfoward one-line change. The code location that invokes the formatter's exported interface function already has access to the API it should use to obtain the list of rules, and the output of that function is a Map (dictionary) which is the type we want to pass to the formatter. This dataset includes all executed rules. + +```js +function printResults(engine, results, format, outputFile) { + let formatter; + + try { + formatter = engine.getFormatter(format); + } catch (e) { + log.error(e.message); + return false; + } + + const output = formatter(results, engine.getRules()); + ... +} +``` + +### Formatter Changes +Though existing formatters, both in-box and custom, will continue to function without modification, we should add the new `rules` parameter to the in-box formatters' exported interface functions. The in-box formatters are canonical examples and represent the best practices for implementation of a formatter. This code also acts as a real-world API sample. + +```js +module.exports = function(results, rules) { + ... +} +``` + +No other changes are required. Future versions of the formatters can make use of the rules data as appropriate. ## Documentation - +Since custom formatter authors may want to take advantage of the newly-available rules data, a formal announcement may be justified (I don't have sufficient context in this regard so I will defer this determination.) + +The [Working with Custom Formatters](https://eslint.org/docs/developer-guide/working-with-custom-formatters) article will have to be updated: +* Code samples will need the new `rules` parameter added wherever the exported interface function is shown. +* The `rules` parameter should be called out and described, and should include a link to the [Working with Rules](https://eslint.org/docs/developer-guide/working-with-rules) article. The primary goal here is to familiarize the formatter author with the structure of the Rule object. +* It should be noted that metadata properties such as description, category, and help URL are not required and may not be defined, and that custom formatter code should take this into account. +* The [Detailed formatter](https://eslint.org/docs/developer-guide/working-with-custom-formatters#detailed-formatter) example should make use of the rules data. One idea would be to suffix the existing output with a list of rules that were violated, using a helper function something like this: + +```js +var rulesViolated = ""; // This is a dictionary that contains the violated rules +function printRules() { + var lines = "*** RULES:\n"; + rulesViolated.forEach(function (rule) { + lines += rule.ruleId; + + if (rule.meta.docs.description) { + lines += ": " + rule.meta.docs.description; + } + + lines += "\n"; + + if (rule.meta.docs.url) { + lines += rule.meta.docs.url + "\n"; + } + }); + return lines; +} +``` ## Drawbacks - +This is a fairly innocuous change in that it is additive, non-breaking, and does not change any of ESLint's core functionality. A downside is that we will be exposing the Rule data model to third-party developers, so future changes could break existing formatters. For example, removing or renaming an existing property, or changing the structure of the Rule object. ## Backwards Compatibility Analysis - +Since this change is manifested as a new parameter to the formatter's exported interface function, existing formatter code will not be affected. Existing formatters will continue to function even without adding the new parameter to their exported function. I have confirmed this assertion via manual testing. ## Alternatives @@ -73,15 +109,11 @@ outcome? --> you've received the answers and updated the design to reflect them, you can remove this section. --> +* Is it possible for a formatter to be called even though no rules have been run? IOW, could the caller suppress the inbox rules without providing any custom rules? ## Help Needed - +No help needed, I have implemented the change. ## Frequently Asked Questions @@ -95,9 +127,11 @@ outcome? --> ## Related Discussions - +Earlier related issue: +https://github.com/eslint/eslint/issues/9841 + +Initial inquiry: +https://groups.google.com/forum/#!topic/eslint/kpHrxkeilwE From 9be43d1cc43bea3d0aff9ea7293e66096ec282cc Mon Sep 17 00:00:00 2001 From: Chris Meyer <32752981+EasyRhinoMSFT@users.noreply.github.com> Date: Thu, 17 Jan 2019 12:46:59 -0800 Subject: [PATCH 06/22] Tweaks --- designs/2019-expose-rules-to-formatters/readme.MD | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/designs/2019-expose-rules-to-formatters/readme.MD b/designs/2019-expose-rules-to-formatters/readme.MD index 39652b31..839d69d2 100644 --- a/designs/2019-expose-rules-to-formatters/readme.MD +++ b/designs/2019-expose-rules-to-formatters/readme.MD @@ -15,7 +15,7 @@ Currently, formatters only see the ID of each rule for which a violation was ide ## Detailed Design Design Summary -1. In cli.js::printResults, obtain the rules map from the Engine object. +1. In `cli.js::printResults`, obtain the rules map from the `Engine` object. 2. Pass the rules map as a new, second parameter to the formatter's exported interface function. 3. Update the in-box formatters by adding the new `rules` parameter to their exported interface function definitions. From 21f470da3f29ae0e0ec8315c7914435e3b9e708d Mon Sep 17 00:00:00 2001 From: Chris Meyer <32752981+EasyRhinoMSFT@users.noreply.github.com> Date: Thu, 17 Jan 2019 12:55:00 -0800 Subject: [PATCH 07/22] Update readme.MD --- designs/2019-expose-rules-to-formatters/readme.MD | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/designs/2019-expose-rules-to-formatters/readme.MD b/designs/2019-expose-rules-to-formatters/readme.MD index 839d69d2..554b57c4 100644 --- a/designs/2019-expose-rules-to-formatters/readme.MD +++ b/designs/2019-expose-rules-to-formatters/readme.MD @@ -57,10 +57,12 @@ The [Working with Custom Formatters](https://eslint.org/docs/developer-guide/wor * Code samples will need the new `rules` parameter added wherever the exported interface function is shown. * The `rules` parameter should be called out and described, and should include a link to the [Working with Rules](https://eslint.org/docs/developer-guide/working-with-rules) article. The primary goal here is to familiarize the formatter author with the structure of the Rule object. * It should be noted that metadata properties such as description, category, and help URL are not required and may not be defined, and that custom formatter code should take this into account. -* The [Detailed formatter](https://eslint.org/docs/developer-guide/working-with-custom-formatters#detailed-formatter) example should make use of the rules data. One idea would be to suffix the existing output with a list of rules that were violated, using a helper function something like this: +* We should show the use of rule data in one of the examples by either modifying an existing one (maybe the [Detailed formatter](https://eslint.org/docs/developer-guide/working-with-custom-formatters#detailed-formatter) example) or adding a new one. One idea would be to suffix the results output with a list of rules that were violated, using a helper function something like this: ```js -var rulesViolated = ""; // This is a dictionary that contains the violated rules +var rulesViolated = ""; // This is a dictionary that contains the violated rules. + // Your code would populate this object as you loop through the results. + // Example: rulesViolated["my-rule"] = theRule; function printRules() { var lines = "*** RULES:\n"; rulesViolated.forEach(function (rule) { @@ -79,6 +81,7 @@ function printRules() { return lines; } ``` +*The explanation of the rulesViolated object needs work.* ## Drawbacks From 4deed2c6ddc301bef8ed714f35b44a2ff5811b9e Mon Sep 17 00:00:00 2001 From: Chris Meyer <32752981+EasyRhinoMSFT@users.noreply.github.com> Date: Thu, 17 Jan 2019 13:23:56 -0800 Subject: [PATCH 08/22] Update readme.MD --- designs/2019-expose-rules-to-formatters/readme.MD | 1 + 1 file changed, 1 insertion(+) diff --git a/designs/2019-expose-rules-to-formatters/readme.MD b/designs/2019-expose-rules-to-formatters/readme.MD index 554b57c4..2b6634a9 100644 --- a/designs/2019-expose-rules-to-formatters/readme.MD +++ b/designs/2019-expose-rules-to-formatters/readme.MD @@ -82,6 +82,7 @@ function printRules() { } ``` *The explanation of the rulesViolated object needs work.* +*Maybe an isolated example?* ## Drawbacks From c769148b417a68235138236312e54945d369a26d Mon Sep 17 00:00:00 2001 From: Chris Meyer <32752981+EasyRhinoMSFT@users.noreply.github.com> Date: Thu, 17 Jan 2019 16:48:03 -0800 Subject: [PATCH 09/22] Update readme.MD --- .../2019-expose-rules-to-formatters/readme.MD | 18 ++++++------------ 1 file changed, 6 insertions(+), 12 deletions(-) diff --git a/designs/2019-expose-rules-to-formatters/readme.MD b/designs/2019-expose-rules-to-formatters/readme.MD index 2b6634a9..d7fd521d 100644 --- a/designs/2019-expose-rules-to-formatters/readme.MD +++ b/designs/2019-expose-rules-to-formatters/readme.MD @@ -17,44 +17,38 @@ Currently, formatters only see the ID of each rule for which a violation was ide Design Summary 1. In `cli.js::printResults`, obtain the rules map from the `Engine` object. 2. Pass the rules map as a new, second parameter to the formatter's exported interface function. -3. Update the in-box formatters by adding the new `rules` parameter to their exported interface function definitions. ### Command Line Interface (cli.js) Changes -The implementation of this feature is exceedingly simple, consisting of a straightfoward one-line change. The code location that invokes the formatter's exported interface function already has access to the API it should use to obtain the list of rules, and the output of that function is a Map (dictionary) which is the type we want to pass to the formatter. This dataset includes all executed rules. +The implementation of this feature is very simple and straightfoward. The code location that invokes the formatter's exported interface function already has access to the API it should use to obtain the list of rules, and the output of that function is a Map (dictionary) which is the type we want to pass to the formatter. This dataset includes all executed rules. The call to `Engine.getRules` must be made in the try block because `enginge` may be null during unit testing. ```js function printResults(engine, results, format, outputFile) { let formatter; + let rules; try { formatter = engine.getFormatter(format); + rules = engine.getRules(); } catch (e) { log.error(e.message); return false; } - const output = formatter(results, engine.getRules()); + const output = formatter(results, rules); ... } ``` ### Formatter Changes -Though existing formatters, both in-box and custom, will continue to function without modification, we should add the new `rules` parameter to the in-box formatters' exported interface functions. The in-box formatters are canonical examples and represent the best practices for implementation of a formatter. This code also acts as a real-world API sample. -```js -module.exports = function(results, rules) { - ... -} -``` - -No other changes are required. Future versions of the formatters can make use of the rules data as appropriate. +No changes to the existing in-box formatters are necessary. Future versions can make use of the rules data by adding the new parameter to the exported interface function definition. This parameter cannot be added unless it is used, as this will trip the JavaScript validation rule 'no-unused-vars.' ## Documentation Since custom formatter authors may want to take advantage of the newly-available rules data, a formal announcement may be justified (I don't have sufficient context in this regard so I will defer this determination.) The [Working with Custom Formatters](https://eslint.org/docs/developer-guide/working-with-custom-formatters) article will have to be updated: -* Code samples will need the new `rules` parameter added wherever the exported interface function is shown. +* Code samples will need the new `rules` parameter added wherever the exported interface function is shown, *but only when it is used*. * The `rules` parameter should be called out and described, and should include a link to the [Working with Rules](https://eslint.org/docs/developer-guide/working-with-rules) article. The primary goal here is to familiarize the formatter author with the structure of the Rule object. * It should be noted that metadata properties such as description, category, and help URL are not required and may not be defined, and that custom formatter code should take this into account. * We should show the use of rule data in one of the examples by either modifying an existing one (maybe the [Detailed formatter](https://eslint.org/docs/developer-guide/working-with-custom-formatters#detailed-formatter) example) or adding a new one. One idea would be to suffix the results output with a list of rules that were violated, using a helper function something like this: From 7bc936a85bd0178719009e7d69570af8a4ebd9a9 Mon Sep 17 00:00:00 2001 From: Chris Meyer <32752981+EasyRhinoMSFT@users.noreply.github.com> Date: Mon, 28 Jan 2019 16:50:08 -0800 Subject: [PATCH 10/22] Update readme.MD --- designs/2019-expose-rules-to-formatters/readme.MD | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/designs/2019-expose-rules-to-formatters/readme.MD b/designs/2019-expose-rules-to-formatters/readme.MD index d7fd521d..bc61158e 100644 --- a/designs/2019-expose-rules-to-formatters/readme.MD +++ b/designs/2019-expose-rules-to-formatters/readme.MD @@ -16,7 +16,9 @@ Currently, formatters only see the ID of each rule for which a violation was ide Design Summary 1. In `cli.js::printResults`, obtain the rules map from the `Engine` object. -2. Pass the rules map as a new, second parameter to the formatter's exported interface function. +2. Pass the rules map as property in a new, second parameter object to the formatter's exported interface function. + +We should use a container object as the parameter, with the rules map as a property, in order to accommodate potential future expansions of the data we pass to formatters. This suggestion was previously made in the discussion of issue #9841. ### Command Line Interface (cli.js) Changes The implementation of this feature is very simple and straightfoward. The code location that invokes the formatter's exported interface function already has access to the API it should use to obtain the list of rules, and the output of that function is a Map (dictionary) which is the type we want to pass to the formatter. This dataset includes all executed rules. The call to `Engine.getRules` must be made in the try block because `enginge` may be null during unit testing. @@ -34,7 +36,7 @@ function printResults(engine, results, format, outputFile) { return false; } - const output = formatter(results, rules); + const output = formatter(results, { rules: rules }); ... } ``` @@ -48,8 +50,8 @@ No changes to the existing in-box formatters are necessary. Future versions can Since custom formatter authors may want to take advantage of the newly-available rules data, a formal announcement may be justified (I don't have sufficient context in this regard so I will defer this determination.) The [Working with Custom Formatters](https://eslint.org/docs/developer-guide/working-with-custom-formatters) article will have to be updated: -* Code samples will need the new `rules` parameter added wherever the exported interface function is shown, *but only when it is used*. -* The `rules` parameter should be called out and described, and should include a link to the [Working with Rules](https://eslint.org/docs/developer-guide/working-with-rules) article. The primary goal here is to familiarize the formatter author with the structure of the Rule object. +* Code samples will need the new `data` parameter added wherever the exported interface function is shown, *but only when it is used*. +* The `data` parameter should be called out and described, and should include a link to the [Working with Rules](https://eslint.org/docs/developer-guide/working-with-rules) article. The primary goal here is to familiarize the formatter author with the structure of the `data` parameter and Rule object. * It should be noted that metadata properties such as description, category, and help URL are not required and may not be defined, and that custom formatter code should take this into account. * We should show the use of rule data in one of the examples by either modifying an existing one (maybe the [Detailed formatter](https://eslint.org/docs/developer-guide/working-with-custom-formatters#detailed-formatter) example) or adding a new one. One idea would be to suffix the results output with a list of rules that were violated, using a helper function something like this: @@ -94,6 +96,8 @@ Since this change is manifested as a new parameter to the formatter's exported i This section should also include prior art, such as whether similar projects have already implemented a similar feature. --> +* Including the rule metadata in the result object. This approach results in redundant data being returned, and includes external metadata properties that are not directly relevant. (As an analogy, it's like ...) +* Pass the rules map itself as a parameter to the formatter's exported interface function. This approach makes it messier to add additional data in the future, since new properties would be necessary. ## Open Questions @@ -107,7 +111,8 @@ Since this change is manifested as a new parameter to the formatter's exported i you've received the answers and updated the design to reflect them, you can remove this section. --> -* Is it possible for a formatter to be called even though no rules have been run? IOW, could the caller suppress the inbox rules without providing any custom rules? +* Is it possible for a formatter to be invoked even though no rules have been run? IOW, could the caller suppress the inbox rules without providing any custom rules? The rules collection would be empty in this case, which formatters could potentially mishandle. +* Is there any opportunity for malicious manipulation of the rule data? I think not, since the analysis has completed by the time the formatter is invoked. ## Help Needed From df56da5d9eeb7537f2d0d5b835c9ac5ce2e6c787 Mon Sep 17 00:00:00 2001 From: Chris Meyer <32752981+EasyRhinoMSFT@users.noreply.github.com> Date: Tue, 29 Jan 2019 11:06:00 -0800 Subject: [PATCH 11/22] Update readme.MD --- designs/2019-expose-rules-to-formatters/readme.MD | 15 ++++++--------- 1 file changed, 6 insertions(+), 9 deletions(-) diff --git a/designs/2019-expose-rules-to-formatters/readme.MD b/designs/2019-expose-rules-to-formatters/readme.MD index bc61158e..5089f3e4 100644 --- a/designs/2019-expose-rules-to-formatters/readme.MD +++ b/designs/2019-expose-rules-to-formatters/readme.MD @@ -51,14 +51,13 @@ Since custom formatter authors may want to take advantage of the newly-available The [Working with Custom Formatters](https://eslint.org/docs/developer-guide/working-with-custom-formatters) article will have to be updated: * Code samples will need the new `data` parameter added wherever the exported interface function is shown, *but only when it is used*. -* The `data` parameter should be called out and described, and should include a link to the [Working with Rules](https://eslint.org/docs/developer-guide/working-with-rules) article. The primary goal here is to familiarize the formatter author with the structure of the `data` parameter and Rule object. -* It should be noted that metadata properties such as description, category, and help URL are not required and may not be defined, and that custom formatter code should take this into account. +* The `data` parameter should be called out and described, and should include a link to the [Working with Rules](https://eslint.org/docs/developer-guide/working-with-rules) article. The primary goal here is to familiarize formatter author with the structure of the `data` parameter and Rule object. +* It should be noted that Rule metadata properties such as description, category, and help URL are not required and may not be defined, and that custom formatter code should take this into account. * We should show the use of rule data in one of the examples by either modifying an existing one (maybe the [Detailed formatter](https://eslint.org/docs/developer-guide/working-with-custom-formatters#detailed-formatter) example) or adding a new one. One idea would be to suffix the results output with a list of rules that were violated, using a helper function something like this: ```js -var rulesViolated = ""; // This is a dictionary that contains the violated rules. - // Your code would populate this object as you loop through the results. - // Example: rulesViolated["my-rule"] = theRule; +var rulesViolated = {}; +... function printRules() { var lines = "*** RULES:\n"; rulesViolated.forEach(function (rule) { @@ -77,8 +76,6 @@ function printRules() { return lines; } ``` -*The explanation of the rulesViolated object needs work.* -*Maybe an isolated example?* ## Drawbacks @@ -86,7 +83,7 @@ This is a fairly innocuous change in that it is additive, non-breaking, and does ## Backwards Compatibility Analysis -Since this change is manifested as a new parameter to the formatter's exported interface function, existing formatter code will not be affected. Existing formatters will continue to function even without adding the new parameter to their exported function. I have confirmed this assertion via manual testing. +Since this change is manifested as a new parameter to the formatter's exported interface function, existing formatter code will not be affected and will continue to function even without adding the new parameter to their exported function. I have confirmed this assertion via manual testing. ## Alternatives @@ -97,7 +94,7 @@ Since this change is manifested as a new parameter to the formatter's exported i projects have already implemented a similar feature. --> * Including the rule metadata in the result object. This approach results in redundant data being returned, and includes external metadata properties that are not directly relevant. (As an analogy, it's like ...) -* Pass the rules map itself as a parameter to the formatter's exported interface function. This approach makes it messier to add additional data in the future, since new properties would be necessary. +* Pass the rules map itself as a parameter to the formatter's exported interface function. This approach makes it messier to add additional data in the future, since new parameters would be necessary. ## Open Questions From 3b935eec3bb46aa92b1836101133141bf40e963b Mon Sep 17 00:00:00 2001 From: Chris Meyer <32752981+EasyRhinoMSFT@users.noreply.github.com> Date: Tue, 29 Jan 2019 11:08:28 -0800 Subject: [PATCH 12/22] Update readme.MD --- designs/2019-expose-rules-to-formatters/readme.MD | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/designs/2019-expose-rules-to-formatters/readme.MD b/designs/2019-expose-rules-to-formatters/readme.MD index 5089f3e4..0a71ec48 100644 --- a/designs/2019-expose-rules-to-formatters/readme.MD +++ b/designs/2019-expose-rules-to-formatters/readme.MD @@ -18,7 +18,7 @@ Design Summary 1. In `cli.js::printResults`, obtain the rules map from the `Engine` object. 2. Pass the rules map as property in a new, second parameter object to the formatter's exported interface function. -We should use a container object as the parameter, with the rules map as a property, in order to accommodate potential future expansions of the data we pass to formatters. This suggestion was previously made in the discussion of issue #9841. +We should use a container object as the parameter, with the rules map as a property, in order to accommodate potential future expansions of the data we pass to formatters. This suggestion was previously made in the discussion of issue [#9841](https://github.com/eslint/eslint/issues/9841). ### Command Line Interface (cli.js) Changes The implementation of this feature is very simple and straightfoward. The code location that invokes the formatter's exported interface function already has access to the API it should use to obtain the list of rules, and the output of that function is a Map (dictionary) which is the type we want to pass to the formatter. This dataset includes all executed rules. The call to `Engine.getRules` must be made in the try block because `enginge` may be null during unit testing. From cf257fc71bcd57ae46a46db544e3113085b71bd9 Mon Sep 17 00:00:00 2001 From: Teddy Katz Date: Tue, 29 Jan 2019 12:40:54 -0800 Subject: [PATCH 13/22] Update designs/2019-expose-rules-to-formatters/readme.MD Co-Authored-By: EasyRhinoMSFT <32752981+EasyRhinoMSFT@users.noreply.github.com> --- designs/2019-expose-rules-to-formatters/readme.MD | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/designs/2019-expose-rules-to-formatters/readme.MD b/designs/2019-expose-rules-to-formatters/readme.MD index 0a71ec48..798cbfbc 100644 --- a/designs/2019-expose-rules-to-formatters/readme.MD +++ b/designs/2019-expose-rules-to-formatters/readme.MD @@ -10,7 +10,7 @@ This proposal describes a design enhancement that provides formatters with detai ## Motivation -Currently, formatters only see the ID of each rule for which a violation was identified, plus an instance-specific description, as properties on each result object. Formatters are not able to access usefule rule metadata, such as category, description, and help URL. Formatters are also not aware of the full set of rules that were run, information that may be useful in some cases. +Currently, formatters only see the ID of each rule for which a violation was identified, plus an instance-specific description, as properties on each result object. Formatters are not able to access useful rule metadata, such as category, description, and help URL. Formatters are also not aware of the full set of rules that were run, information that may be useful in some cases. ## Detailed Design From 2129d3f3fb7744a87e1b0dd5fa984916f5702280 Mon Sep 17 00:00:00 2001 From: Teddy Katz Date: Tue, 29 Jan 2019 12:41:36 -0800 Subject: [PATCH 14/22] Update designs/2019-expose-rules-to-formatters/readme.MD Co-Authored-By: EasyRhinoMSFT <32752981+EasyRhinoMSFT@users.noreply.github.com> --- designs/2019-expose-rules-to-formatters/readme.MD | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/designs/2019-expose-rules-to-formatters/readme.MD b/designs/2019-expose-rules-to-formatters/readme.MD index 798cbfbc..f3569888 100644 --- a/designs/2019-expose-rules-to-formatters/readme.MD +++ b/designs/2019-expose-rules-to-formatters/readme.MD @@ -21,7 +21,7 @@ Design Summary We should use a container object as the parameter, with the rules map as a property, in order to accommodate potential future expansions of the data we pass to formatters. This suggestion was previously made in the discussion of issue [#9841](https://github.com/eslint/eslint/issues/9841). ### Command Line Interface (cli.js) Changes -The implementation of this feature is very simple and straightfoward. The code location that invokes the formatter's exported interface function already has access to the API it should use to obtain the list of rules, and the output of that function is a Map (dictionary) which is the type we want to pass to the formatter. This dataset includes all executed rules. The call to `Engine.getRules` must be made in the try block because `enginge` may be null during unit testing. +The implementation of this feature is very simple and straightfoward. The code location that invokes the formatter's exported interface function already has access to the API it should use to obtain the list of rules, and the output of that function is a Map (dictionary) which is the type we want to pass to the formatter. This dataset includes all executed rules. The call to `Engine.getRules` must be made in the try block because `engine` may be null during unit testing. ```js function printResults(engine, results, format, outputFile) { From 17aaa93de06bf5665d5c8bb838ad047af4332cfb Mon Sep 17 00:00:00 2001 From: Chris Meyer <32752981+EasyRhinoMSFT@users.noreply.github.com> Date: Tue, 29 Jan 2019 16:23:07 -0800 Subject: [PATCH 15/22] PR feedback --- .../2019-expose-rules-to-formatters/readme.MD | 75 ++++++++++++++----- 1 file changed, 55 insertions(+), 20 deletions(-) diff --git a/designs/2019-expose-rules-to-formatters/readme.MD b/designs/2019-expose-rules-to-formatters/readme.MD index f3569888..5120ecd0 100644 --- a/designs/2019-expose-rules-to-formatters/readme.MD +++ b/designs/2019-expose-rules-to-formatters/readme.MD @@ -16,12 +16,25 @@ Currently, formatters only see the ID of each rule for which a violation was ide Design Summary 1. In `cli.js::printResults`, obtain the rules map from the `Engine` object. -2. Pass the rules map as property in a new, second parameter object to the formatter's exported interface function. +2. Add a second argument to the formatter's exported interface function. The value should be an object with a `ruleMetadata` property that is a map with the rule name as the key and the `rule.meta` object as the value. -We should use a container object as the parameter, with the rules map as a property, in order to accommodate potential future expansions of the data we pass to formatters. This suggestion was previously made in the discussion of issue [#9841](https://github.com/eslint/eslint/issues/9841). +```js +const output = formatter( + results, + { + ruleMetadata: { + "accessor-pairs": rules.get("accessor-pairs").meta, + "array-bracket-newline": rules.get("array-bracket-newline").meta, + // ... + } + } +); +``` + +We should use a container object as the argument, with a ruleId/rule.meta map as a property, in order to accommodate potential future expansions of the data we pass to formatters. This suggestion was previously made in the discussion of issue [#9841](https://github.com/eslint/eslint/issues/9841). ### Command Line Interface (cli.js) Changes -The implementation of this feature is very simple and straightfoward. The code location that invokes the formatter's exported interface function already has access to the API it should use to obtain the list of rules, and the output of that function is a Map (dictionary) which is the type we want to pass to the formatter. This dataset includes all executed rules. The call to `Engine.getRules` must be made in the try block because `engine` may be null during unit testing. +The implementation of this feature is very simple and straightfoward. The code location that invokes the formatter's exported interface function already has access to the API it should use to obtain the list of all executed rules. The call to `Engine.getRules` must be made in the try block because `engine` may be null during unit testing. ```js function printResults(engine, results, format, outputFile) { @@ -36,41 +49,60 @@ function printResults(engine, results, format, outputFile) { return false; } - const output = formatter(results, { rules: rules }); + const rulesMetadata = {}; + rules.forEach(function(rule, ruleId) { + rulesMetadata[ruleId] = rule.meta; + }); + const output = formatter(results, { ruleMetdata: rulesMetadata }); ... } ``` ### Formatter Changes -No changes to the existing in-box formatters are necessary. Future versions can make use of the rules data by adding the new parameter to the exported interface function definition. This parameter cannot be added unless it is used, as this will trip the JavaScript validation rule 'no-unused-vars.' +Formatters that implement the exported interface function would no changes. Future versions can make use of the rules data by adding the new argument to the exported interface function definition. This argument cannot be added unless it is used, as this will trip the JavaScript validation rule 'no-unused-vars.' + +A formatter that assigns a function reference to the exported interface function could exhibit unexpected behavior depending on the signature of the referenced function. For example, since this change's second argument is a complex object, a referenced function that expects a Number as its second argument could cause an exception. + +Currently the `html` formatter creates incorrect links rooted at the eslint.org domain for rules from plugins. We should fix this issue by using the meta.docs.url property that will become available with this change. + +The `json` formatter also requires attention. It simply stringifies the `results` object, and would therefore provide incomplete data by ignoring the new `data` argument. To avoid a breaking change to the existing `json` formatter, we should a new built-in formatter, perhaps named `json-with-metadata`, which returns a stringified object containing both objects: + +```js +module.exports = function(results, data) { + return JSON.stringify({ + results: results, + data: data + }); +}; +``` ## Documentation -Since custom formatter authors may want to take advantage of the newly-available rules data, a formal announcement may be justified (I don't have sufficient context in this regard so I will defer this determination.) +Since custom formatter authors may want to take advantage of the newly-available rule metadata, a formal announcement may be justified (I don't have sufficient context in this regard so I will defer this determination.) The [Working with Custom Formatters](https://eslint.org/docs/developer-guide/working-with-custom-formatters) article will have to be updated: -* Code samples will need the new `data` parameter added wherever the exported interface function is shown, *but only when it is used*. -* The `data` parameter should be called out and described, and should include a link to the [Working with Rules](https://eslint.org/docs/developer-guide/working-with-rules) article. The primary goal here is to familiarize formatter author with the structure of the `data` parameter and Rule object. -* It should be noted that Rule metadata properties such as description, category, and help URL are not required and may not be defined, and that custom formatter code should take this into account. -* We should show the use of rule data in one of the examples by either modifying an existing one (maybe the [Detailed formatter](https://eslint.org/docs/developer-guide/working-with-custom-formatters#detailed-formatter) example) or adding a new one. One idea would be to suffix the results output with a list of rules that were violated, using a helper function something like this: +* Code samples will need the new `data` argument added wherever the exported interface function is shown, *but only when it is used*. +* The `data` argument should be called out and described, and should include a link to the [Working with Rules](https://eslint.org/docs/developer-guide/working-with-rules) article. The primary goal here is to familiarize formatter author with the structure of the `data` argument and ruleMetadata object. +* It should be noted that rule metadata properties such as description, category, and help URL are not required and may not be defined, and that custom formatter code should take this into account. +* We should show the use of rule metadata in one of the examples by either modifying an existing one (maybe the [Detailed formatter](https://eslint.org/docs/developer-guide/working-with-custom-formatters#detailed-formatter) example) or adding a new one. One idea would be to suffix the results output with a list of rules that were violated, using a helper function something like this: ```js var rulesViolated = {}; ... function printRules() { var lines = "*** RULES:\n"; - rulesViolated.forEach(function (rule) { - lines += rule.ruleId; + rulesViolated.forEach(function (ruleMetadata, ruleId) { + lines += ruleId; - if (rule.meta.docs.description) { - lines += ": " + rule.meta.docs.description; + if (ruleMetadata.docs.description) { + lines += ": " + ruleMetadata.docs.description; } lines += "\n"; - if (rule.meta.docs.url) { - lines += rule.meta.docs.url + "\n"; + if (ruleMetadata.docs.url) { + lines += ruleMetadata.docs.url + "\n"; } }); return lines; @@ -79,11 +111,14 @@ function printRules() { ## Drawbacks -This is a fairly innocuous change in that it is additive, non-breaking, and does not change any of ESLint's core functionality. A downside is that we will be exposing the Rule data model to third-party developers, so future changes could break existing formatters. For example, removing or renaming an existing property, or changing the structure of the Rule object. +This is a fairly innocuous change in that it is additive, non-breaking (mostly, see Backwards Compatibility), and does not change any of ESLint's core functionality. A downside is that we will be exposing the Rule data model to third-party developers, so future changes could break existing formatters. For example, removing or renaming an existing property, or changing the structure of the Rule.meta object. ## Backwards Compatibility Analysis -Since this change is manifested as a new parameter to the formatter's exported interface function, existing formatter code will not be affected and will continue to function even without adding the new parameter to their exported function. I have confirmed this assertion via manual testing. +Since this change is manifested as a new argument to the formatter's exported interface function, existing formatter code that implements the exported interface function will not be affected and will continue to function even without adding the new argument to their exported function. + +(The following paragraph also appears in the Formatters section.) +A formatter that assigns a function reference to the exported interface function could exhibit unexpected behavior depending on the signature of the referenced function. For example, since this change's second argument is a complex object, a referenced function that expects a Number as its second argument could cause an exception. ## Alternatives @@ -93,8 +128,8 @@ Since this change is manifested as a new parameter to the formatter's exported i This section should also include prior art, such as whether similar projects have already implemented a similar feature. --> -* Including the rule metadata in the result object. This approach results in redundant data being returned, and includes external metadata properties that are not directly relevant. (As an analogy, it's like ...) -* Pass the rules map itself as a parameter to the formatter's exported interface function. This approach makes it messier to add additional data in the future, since new parameters would be necessary. +* Including the rule metadata in the result object. This approach results in redundant data being returned, and includes external metadata properties that are not directly relevant. +* Pass the rules map itself as a argument to the formatter's exported interface function. This approach makes it messier to add additional data in the future, since new arguments would be necessary. ## Open Questions From a660fbd90b3946fe91d1d258e447ba9f65c01c74 Mon Sep 17 00:00:00 2001 From: Chris Meyer <32752981+EasyRhinoMSFT@users.noreply.github.com> Date: Wed, 30 Jan 2019 09:37:46 -0800 Subject: [PATCH 16/22] Tweaks --- .../2019-expose-rules-to-formatters/readme.MD | 17 ++--------------- 1 file changed, 2 insertions(+), 15 deletions(-) diff --git a/designs/2019-expose-rules-to-formatters/readme.MD b/designs/2019-expose-rules-to-formatters/readme.MD index 5120ecd0..77e3d01f 100644 --- a/designs/2019-expose-rules-to-formatters/readme.MD +++ b/designs/2019-expose-rules-to-formatters/readme.MD @@ -16,20 +16,7 @@ Currently, formatters only see the ID of each rule for which a violation was ide Design Summary 1. In `cli.js::printResults`, obtain the rules map from the `Engine` object. -2. Add a second argument to the formatter's exported interface function. The value should be an object with a `ruleMetadata` property that is a map with the rule name as the key and the `rule.meta` object as the value. - -```js -const output = formatter( - results, - { - ruleMetadata: { - "accessor-pairs": rules.get("accessor-pairs").meta, - "array-bracket-newline": rules.get("array-bracket-newline").meta, - // ... - } - } -); -``` +2. Add a second argument to the formatter's exported interface function. The value should be an object with a `ruleMetadata` property that is a map with the rule name as the key and the `rule.meta` object as the value. See the "Command Line Interface (cli.js) Changes" section below for implementation. We should use a container object as the argument, with a ruleId/rule.meta map as a property, in order to accommodate potential future expansions of the data we pass to formatters. This suggestion was previously made in the discussion of issue [#9841](https://github.com/eslint/eslint/issues/9841). @@ -53,7 +40,7 @@ function printResults(engine, results, format, outputFile) { rules.forEach(function(rule, ruleId) { rulesMetadata[ruleId] = rule.meta; }); - const output = formatter(results, { ruleMetdata: rulesMetadata }); + const output = formatter(results, { ruleMetadata: rulesMetadata }); ... } ``` From 27b2f701b0c4d4ebe4cf5db18dd9fe7d4d536a4c Mon Sep 17 00:00:00 2001 From: Chris Meyer <32752981+EasyRhinoMSFT@users.noreply.github.com> Date: Wed, 30 Jan 2019 09:44:35 -0800 Subject: [PATCH 17/22] Update readme.MD --- designs/2019-expose-rules-to-formatters/readme.MD | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/designs/2019-expose-rules-to-formatters/readme.MD b/designs/2019-expose-rules-to-formatters/readme.MD index 77e3d01f..461a7bc3 100644 --- a/designs/2019-expose-rules-to-formatters/readme.MD +++ b/designs/2019-expose-rules-to-formatters/readme.MD @@ -56,10 +56,10 @@ Currently the `html` formatter creates incorrect links rooted at the eslint.org The `json` formatter also requires attention. It simply stringifies the `results` object, and would therefore provide incomplete data by ignoring the new `data` argument. To avoid a breaking change to the existing `json` formatter, we should a new built-in formatter, perhaps named `json-with-metadata`, which returns a stringified object containing both objects: ```js -module.exports = function(results, data) { +module.exports = function(results, ruleMetadata) { return JSON.stringify({ results: results, - data: data + ruleMetadata: ruleMetadata }); }; ``` From 7f16868ac33541481cec3bb6166bc03d5a19c615 Mon Sep 17 00:00:00 2001 From: Chris Meyer <32752981+EasyRhinoMSFT@users.noreply.github.com> Date: Wed, 30 Jan 2019 10:26:51 -0800 Subject: [PATCH 18/22] Property name tweaks --- designs/2019-expose-rules-to-formatters/readme.MD | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/designs/2019-expose-rules-to-formatters/readme.MD b/designs/2019-expose-rules-to-formatters/readme.MD index 461a7bc3..3a873a56 100644 --- a/designs/2019-expose-rules-to-formatters/readme.MD +++ b/designs/2019-expose-rules-to-formatters/readme.MD @@ -16,7 +16,7 @@ Currently, formatters only see the ID of each rule for which a violation was ide Design Summary 1. In `cli.js::printResults`, obtain the rules map from the `Engine` object. -2. Add a second argument to the formatter's exported interface function. The value should be an object with a `ruleMetadata` property that is a map with the rule name as the key and the `rule.meta` object as the value. See the "Command Line Interface (cli.js) Changes" section below for implementation. +2. Add a second argument to the formatter's exported interface function. The value should be an object with a `rulesMetadata` property that is a map with the rule name as the key and the `rule.meta` object as the value. See the "Command Line Interface (cli.js) Changes" section below for implementation. We should use a container object as the argument, with a ruleId/rule.meta map as a property, in order to accommodate potential future expansions of the data we pass to formatters. This suggestion was previously made in the discussion of issue [#9841](https://github.com/eslint/eslint/issues/9841). @@ -40,7 +40,7 @@ function printResults(engine, results, format, outputFile) { rules.forEach(function(rule, ruleId) { rulesMetadata[ruleId] = rule.meta; }); - const output = formatter(results, { ruleMetadata: rulesMetadata }); + const output = formatter(results, { rulesMetadata: rulesMetadata }); ... } ``` @@ -56,10 +56,10 @@ Currently the `html` formatter creates incorrect links rooted at the eslint.org The `json` formatter also requires attention. It simply stringifies the `results` object, and would therefore provide incomplete data by ignoring the new `data` argument. To avoid a breaking change to the existing `json` formatter, we should a new built-in formatter, perhaps named `json-with-metadata`, which returns a stringified object containing both objects: ```js -module.exports = function(results, ruleMetadata) { +module.exports = function(results, rulesMetadata) { return JSON.stringify({ results: results, - ruleMetadata: ruleMetadata + rulesMetadata: rulesMetadata }); }; ``` @@ -70,7 +70,7 @@ Since custom formatter authors may want to take advantage of the newly-available The [Working with Custom Formatters](https://eslint.org/docs/developer-guide/working-with-custom-formatters) article will have to be updated: * Code samples will need the new `data` argument added wherever the exported interface function is shown, *but only when it is used*. -* The `data` argument should be called out and described, and should include a link to the [Working with Rules](https://eslint.org/docs/developer-guide/working-with-rules) article. The primary goal here is to familiarize formatter author with the structure of the `data` argument and ruleMetadata object. +* The `data` argument should be called out and described, and should include a link to the [Working with Rules](https://eslint.org/docs/developer-guide/working-with-rules) article. The primary goal here is to familiarize formatter author with the structure of the `data` argument and rulesMetadata property. * It should be noted that rule metadata properties such as description, category, and help URL are not required and may not be defined, and that custom formatter code should take this into account. * We should show the use of rule metadata in one of the examples by either modifying an existing one (maybe the [Detailed formatter](https://eslint.org/docs/developer-guide/working-with-custom-formatters#detailed-formatter) example) or adding a new one. One idea would be to suffix the results output with a list of rules that were violated, using a helper function something like this: From a3e32581b8b29941964b1c0cf9a9f595223f3cf2 Mon Sep 17 00:00:00 2001 From: Chris Meyer <32752981+EasyRhinoMSFT@users.noreply.github.com> Date: Wed, 30 Jan 2019 10:28:05 -0800 Subject: [PATCH 19/22] Update readme.MD --- designs/2019-expose-rules-to-formatters/readme.MD | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/designs/2019-expose-rules-to-formatters/readme.MD b/designs/2019-expose-rules-to-formatters/readme.MD index 3a873a56..26d14cd0 100644 --- a/designs/2019-expose-rules-to-formatters/readme.MD +++ b/designs/2019-expose-rules-to-formatters/readme.MD @@ -56,10 +56,10 @@ Currently the `html` formatter creates incorrect links rooted at the eslint.org The `json` formatter also requires attention. It simply stringifies the `results` object, and would therefore provide incomplete data by ignoring the new `data` argument. To avoid a breaking change to the existing `json` formatter, we should a new built-in formatter, perhaps named `json-with-metadata`, which returns a stringified object containing both objects: ```js -module.exports = function(results, rulesMetadata) { +module.exports = function(results, data) { return JSON.stringify({ results: results, - rulesMetadata: rulesMetadata + rulesMetadata: data.rulesMetadata }); }; ``` From c3897ff11b24952faef8b160d668a8bbac9bf3a1 Mon Sep 17 00:00:00 2001 From: "Nicholas C. Zakas" Date: Fri, 1 Feb 2019 09:54:19 -0800 Subject: [PATCH 20/22] Update designs/2019-expose-rules-to-formatters/readme.MD Co-Authored-By: EasyRhinoMSFT <32752981+EasyRhinoMSFT@users.noreply.github.com> --- designs/2019-expose-rules-to-formatters/readme.MD | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/designs/2019-expose-rules-to-formatters/readme.MD b/designs/2019-expose-rules-to-formatters/readme.MD index 26d14cd0..2273230e 100644 --- a/designs/2019-expose-rules-to-formatters/readme.MD +++ b/designs/2019-expose-rules-to-formatters/readme.MD @@ -1,5 +1,5 @@ - Start Date: 2019-01-16 -- RFC PR: (leave this empty, to be filled in later) +- RFC PR: https://github.com/eslint/rfcs/pull/10 - Authors: Chris Meyer (@EasyRhinoMSFT) # Providing Rule Metadata to Formatters From f53ba0a1d2b795e2975980a38994d63b4ed8a4dc Mon Sep 17 00:00:00 2001 From: Chris Meyer <32752981+EasyRhinoMSFT@users.noreply.github.com> Date: Fri, 1 Feb 2019 09:58:39 -0800 Subject: [PATCH 21/22] Change property name rulesMetadata -> rulesMeta --- designs/2019-expose-rules-to-formatters/readme.MD | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/designs/2019-expose-rules-to-formatters/readme.MD b/designs/2019-expose-rules-to-formatters/readme.MD index 2273230e..12e588c2 100644 --- a/designs/2019-expose-rules-to-formatters/readme.MD +++ b/designs/2019-expose-rules-to-formatters/readme.MD @@ -16,7 +16,7 @@ Currently, formatters only see the ID of each rule for which a violation was ide Design Summary 1. In `cli.js::printResults`, obtain the rules map from the `Engine` object. -2. Add a second argument to the formatter's exported interface function. The value should be an object with a `rulesMetadata` property that is a map with the rule name as the key and the `rule.meta` object as the value. See the "Command Line Interface (cli.js) Changes" section below for implementation. +2. Add a second argument to the formatter's exported interface function. The value should be an object with a `rulesMeta` property that is a map with the rule name as the key and the `rule.meta` object as the value. See the "Command Line Interface (cli.js) Changes" section below for implementation. We should use a container object as the argument, with a ruleId/rule.meta map as a property, in order to accommodate potential future expansions of the data we pass to formatters. This suggestion was previously made in the discussion of issue [#9841](https://github.com/eslint/eslint/issues/9841). @@ -36,11 +36,11 @@ function printResults(engine, results, format, outputFile) { return false; } - const rulesMetadata = {}; + const rulesMeta = {}; rules.forEach(function(rule, ruleId) { - rulesMetadata[ruleId] = rule.meta; + rulesMeta[ruleId] = rule.meta; }); - const output = formatter(results, { rulesMetadata: rulesMetadata }); + const output = formatter(results, { rulesMeta: rulesMeta }); ... } ``` @@ -59,7 +59,7 @@ The `json` formatter also requires attention. It simply stringifies the `results module.exports = function(results, data) { return JSON.stringify({ results: results, - rulesMetadata: data.rulesMetadata + rulesMeta: data.rulesMeta }); }; ``` @@ -70,7 +70,7 @@ Since custom formatter authors may want to take advantage of the newly-available The [Working with Custom Formatters](https://eslint.org/docs/developer-guide/working-with-custom-formatters) article will have to be updated: * Code samples will need the new `data` argument added wherever the exported interface function is shown, *but only when it is used*. -* The `data` argument should be called out and described, and should include a link to the [Working with Rules](https://eslint.org/docs/developer-guide/working-with-rules) article. The primary goal here is to familiarize formatter author with the structure of the `data` argument and rulesMetadata property. +* The `data` argument should be called out and described, and should include a link to the [Working with Rules](https://eslint.org/docs/developer-guide/working-with-rules) article. The primary goal here is to familiarize formatter author with the structure of the `data` argument and rulesMeta property. * It should be noted that rule metadata properties such as description, category, and help URL are not required and may not be defined, and that custom formatter code should take this into account. * We should show the use of rule metadata in one of the examples by either modifying an existing one (maybe the [Detailed formatter](https://eslint.org/docs/developer-guide/working-with-custom-formatters#detailed-formatter) example) or adding a new one. One idea would be to suffix the results output with a list of rules that were violated, using a helper function something like this: From 685317a435bc01781b1e75c1cd8dddd70145e15e Mon Sep 17 00:00:00 2001 From: Chris Meyer <32752981+EasyRhinoMSFT@users.noreply.github.com> Date: Fri, 1 Feb 2019 10:02:07 -0800 Subject: [PATCH 22/22] Open Questions update + remove section --- .../2019-expose-rules-to-formatters/readme.MD | 16 +--------------- 1 file changed, 1 insertion(+), 15 deletions(-) diff --git a/designs/2019-expose-rules-to-formatters/readme.MD b/designs/2019-expose-rules-to-formatters/readme.MD index 12e588c2..6037c578 100644 --- a/designs/2019-expose-rules-to-formatters/readme.MD +++ b/designs/2019-expose-rules-to-formatters/readme.MD @@ -71,6 +71,7 @@ Since custom formatter authors may want to take advantage of the newly-available The [Working with Custom Formatters](https://eslint.org/docs/developer-guide/working-with-custom-formatters) article will have to be updated: * Code samples will need the new `data` argument added wherever the exported interface function is shown, *but only when it is used*. * The `data` argument should be called out and described, and should include a link to the [Working with Rules](https://eslint.org/docs/developer-guide/working-with-rules) article. The primary goal here is to familiarize formatter author with the structure of the `data` argument and rulesMeta property. +* It should be noted that the rulesMeta dictionary will be empty in cases where no rules have been run. * It should be noted that rule metadata properties such as description, category, and help URL are not required and may not be defined, and that custom formatter code should take this into account. * We should show the use of rule metadata in one of the examples by either modifying an existing one (maybe the [Detailed formatter](https://eslint.org/docs/developer-guide/working-with-custom-formatters#detailed-formatter) example) or adding a new one. One idea would be to suffix the results output with a list of rules that were violated, using a helper function something like this: @@ -118,21 +119,6 @@ A formatter that assigns a function reference to the exported interface function * Including the rule metadata in the result object. This approach results in redundant data being returned, and includes external metadata properties that are not directly relevant. * Pass the rules map itself as a argument to the formatter's exported interface function. This approach makes it messier to add additional data in the future, since new arguments would be necessary. -## Open Questions - - -* Is it possible for a formatter to be invoked even though no rules have been run? IOW, could the caller suppress the inbox rules without providing any custom rules? The rules collection would be empty in this case, which formatters could potentially mishandle. -* Is there any opportunity for malicious manipulation of the rule data? I think not, since the analysis has completed by the time the formatter is invoked. - ## Help Needed No help needed, I have implemented the change.