Skip to content

Latest commit

 

History

History
83 lines (49 loc) · 4.01 KB

design.md

File metadata and controls

83 lines (49 loc) · 4.01 KB
Raw
  • Start Date: 2019-06-07
  • RFC PR: #25
  • Authors: Toru Nagashima (@mysticatea)

RuleTester Improvements

Summary

This RFC improves RuleTester to check more mistakes.

Motivation

RuleTester overlooks some mistakes.

  • Using non-standard properties of AST (typescript-eslint/typescript-eslint#405).
    Especially, node.start and node.end exist in AST espree made, but it's not standardized and some custom parsers don't make those properties. But node.loc has start/end properties, so it's hard to detect node.start/node.end with static analysis. Therefore, RuleTester should detect those.
  • Untested autofix.
    If people forgot to write output property in test cases, RuleTester doesn't test autofix silently.
  • errors property with a number (found in eslint/eslint#11798).
    errors property with a number ignores syntax errors in test code. We overlooked the mistake of tests/lib/rules/complexity.js#L84 due to this. The number errors property cannot check the reported error was the expected error.

Detailed Design

  1. Disallowing node.start and node.end
  2. Ensuring to test autofix
  3. Deprecating the errors property with a number

1. Disallowing node.start and node.end

RuleTester fails test cases if a rule implementation used node.start or node.end in the test case.

Implementation

  • In RuleTester, it registers an internal custom parser that wraps espree or the parser of item.parser to Linter object.
  • The internal custom parser fixes the AST that the original parser returned, as like test-parser.js.

2. Ensuring to test autofix

RuleTester fails test cases if a rule implementation fixed code but output property was not defined in the test case.

Implementation

  • If output property didn't exist but the rule fixed the code, RuleTester fails the test case as "The rule fixed the code. Please add 'output' property." It's implemented around lib/rule-tester/rule-tester.js#L594.

3. Deprecating the errors property with a number

ESLint prints a deprecation warning if errors property was used with a number.

Implementation

Documentation

RuleTester should be updated.

  • errors ("number or array" → "array")
  • output ("optional" → "required if the rule fixes code")

Drawbacks

This change may enforce plugin owners to fix their tests.

Backwards Compatibility Analysis

This is a breaking change that can break existing tests.

But the breaking cases may indicate that the rule was not tested enough.

Alternatives

  • About "Disallowing node.start and node.end", we can standardize those properties. But it's a breaking change for custom parser owners. On the other hand, using node.start and node.end breaks the rule if users used custom parsers, so the impact of this disallowing is limited.

Open Questions

Frequently Asked Questions

Related Discussions