Linter 2023 Focus: stability and user experience

[Conaclos]

This is an in-progress report outlining the focus for the next few weeks.

Improve user experience

Promote some suggestions to safe fixes

A suggestion or unsafe fix can change the behavior of the program or the intent of the programmer.
In contrast to suggestions, A safe fix does not change the behavior of the program.
A user can automatically apply a safe fixes with confidence.
From a rule maker point of view is thus scary of marking a fix as a safe fix.
Thus, we generally start with suggestions.

Biome has now more rules with suggestions than safe fixes.
This leads some users to directly apply all unsafe fixes.
As with nursery rules which are promoted to another group once stabilized,
we should promote relevant suggestions to safe fixes once we are confident about them.

I think that the following rules could benefit of the promotion of (some of) their suggestions to safe fixes.

• noMultipleSpacesInRegularExpressionLiterals (delayed because of #383)
• noNegationElse
• noUselessLabel
• noUselessTypeConstraint
• noUnusedLabels
• useConst
• useEnumInitializers
• useExponentiationOperator
• useLiteralKeys
• useNumericLiterals
• useOptionalChain
• useSingleVarDeclarator (delayed because of 💅 useSingleVarDeclarator suggests non-working code #728)
• useTemplate
• useWhile

Provide more code fixes

• noUnusedVariables

  A catch with an unused variable could be shorten to an argument-less catch.

• useIsNan

  We could provide a suggestion.

Smarter rules

• useTemplate

  The rule provides the following suggestion:

  - a + b + "px"
  + `${a}${b}px`

  This breaks code where a and b are numbers (rome#4713).
  To fix the issue, we could make the following suggestion:

  - a + b + "px"
  + `${a + b}px`

  Note that the issue is not present for the following code:

  "s" + a + b

• Remove noAssignInExpressions code fix

  The code fix is sometimes unwanted by users rome#4717

Deprecate annoying or confusing rules

• Remove noConfusingArrow

  The rule is already turned off in most cases.

• useSingleCaseStatement

  The rule is confusing: it requires block around in switch clause if there is more than 1 statement.
  A lot of users complained about the rule.

  We should deprecate the rule and provide a rule that enforces block in all switch clauses.

Improve stability

Write guidelines

Rule promotions. Guideline to promote a rule (when, ...).

Code fixes. It is sometimes hard to decide whether a code fix is safe or unsafe.
Safe fixes are automatically applied with the fix-all and quickfixes on-save IDE actions.
As a result, it is a bit risky of marking a code fix as safe.
Conservatively, we categorize a code fix as unsafe.
Unfortunately, this reduces the usefulness of the code fixes and encourages the use of unsafe fixes.
In this regard, we should provide some guidelines and a clear procedure to update an eligible unsafe fix to a safe fix.

Fix long-standing issues

• noUnreachableSuper bugs (rome#4674 and 4616)

• noUnnecessaryContinue rome#4745

• noRedeclare rome#4291

• noUnusedVariables rome#4245

• useSimplifiedLogicExpression rome#3527

• useTemplate incorrectly handle single-line comments rome#4109

• rome-ignore issue rome#3734

• useExponentiationOperator rome#3850

• Suppress comments and indentation rome#3923

Test rules on big projects

• TypeScript Compiler
• Ant Design
• surcase
• vscode code base
• deno std
• Any idea?

[JoshuaKGoldberg]

Test rules on big projects

Suggestion: test the rules on other AST / static analysis projects, and linting projects (ESLint, typescript-eslint, etc) in particular.

• AST projects tend to have a lot of structures / pieces of syntax that are different from your typical userland ones
• Looking at linters raises visibility for what everyone else is doing, which can be good for feature & edge case ideation

[smorimoto]

My team explicitly enabled this, but I'm still not sure how hard this is to port.
https://typescript-eslint.io/rules/switch-exhaustiveness-check

[ematipico]

It requires type checking, which Biome doesn't have yet unfortunately

[smorimoto]

That's where I was concerned.

[jpike88]

Desperate for this one... would this be a low hanging fruit for when type awareness is brought? I guess it's just checking that all items in the union of the type have a matching case right

[arendjr]

I wouldn’t say it’s exactly trivial, because you may have a switch over something likeobject.type, where object has union type and type has a derived union type. So we’ll need to be at a point where we have derived types at least. But once we have those, this rule shouldn’t be too hard anymore.

Btw, a trick I tend to use today is to make sure switches are always in a standalone function with an explicit return type annotation. This way, I can use return instead of break and TypeScript will complain if I forget a case. Although this assumes undefined is not an acceptable return value, because this strategy doesn’t help if it is.

[smorimoto]

This is very important when implementing software while using Node.js compatibility mode in the Edge Runtime, and if it's supported by default, it's much better.
https://github.com/sindresorhus/eslint-plugin-unicorn/blob/v49.0.0/docs/rules/prefer-node-protocol.md

[Conaclos]

useNodeImportProtocol is implemented and will be available in the upcoming release v1.5.0

[smorimoto]

Superb!!

[smorimoto]

@Conaclos By the way, does it only support ESM? Or does it also support CJS?

[Conaclos]

I also supports require imports. You can try it on the playground.

[smorimoto]

I believe these are good directions for performance and consistency.
https://github.com/sindresorhus/eslint-plugin-unicorn/blob/v49.0.0/docs/rules/prefer-set-has.md
https://github.com/sindresorhus/eslint-plugin-unicorn/blob/v49.0.0/docs/rules/prefer-spread.md

[Conaclos]

Please use the ✅ Linter rules from other sources disussion to propose rules.

[vasucp1207]

I have a question, is it a good idea for rules like useAltText, useButtonType to provide a code fix with an empty string to the missing attribute?

Ex for useAltText,

<img src="image.png" />

After applying code fix

<img src="image.png" alt="" />

[unvalley]

@vasucp1207
I had a similar thought when I implemented noSvgWithoutTitle. If incomplete suggestions are accepted, users who accept without much thought may make the mistake of judging that the problem has been solved. It would be nice if the empty-string state would also produce a lint in the case, because it's still incomplete.

[vasucp1207]

Yeah, checking for empty strings can be very helpful, we should provide a different diagnostic message in that case.

[Conaclos]

It seems confusing to me that a rule applies a fix and then complains about the fix it applied... In this case it is better not to provide any fix. Still, it is a good idea to print a diagnostic for empty strings.

[smorimoto]

I think we should pay attention to these, but the direction is right.

• https://www.w3.org/TR/2016/NOTE-WCAG20-TECHS-20161007/H37
• https://www.w3.org/TR/html401/struct/objects.html#adef-alt
  • Do not specify irrelevant alternate text when including images intended to format a page, for instance, alt="red ball" would be inappropriate for an image that adds a red ball for decorating a heading or paragraph. In such cases, the alternate text should be the empty string (""). Authors are in any case advised to avoid using images to format pages; style sheets should be used instead.
  • Do not specify meaningless alternate text (e.g., "dummy text"). Not only will this frustrate users, it will slow down user agents that must convert text to speech or braille output.

[vasucp1207]

Right providing a fix doesn't make sense if it's not going to solve the problem completely. But the diagnostic can help.