feat(lint/noEvolvingAny): add rule #2112
Conversation
github-actions
bot
added
A-Project
✅ Deploy Preview for biomejs ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
CodSpeed Performance ReportMerging #2112 will not alter performanceFalling back to comparing Summary
|
|
@ematipico @Conaclos |
| } | ||
|
|
||
| if is_initialized { | ||
| let initializer = variable.initializer().unwrap(); |
There was a problem hiding this comment.
| let initializer = variable.initializer().unwrap(); | |
| let initializer = variable.initializer()?; |
| let optional_array_expression = expression.as_js_array_expression(); | ||
| let optional_js_literal_expression = expression.as_any_js_literal_expression(); | ||
|
|
||
| if let Some(array_expression) = optional_array_expression { | ||
| if array_expression.elements().into_iter().next().is_none() | ||
| && !is_type_annotated | ||
| { | ||
| return Some(variable); | ||
| } | ||
| } | ||
|
|
||
| if let Some(js_literal_expression) = optional_js_literal_expression { | ||
| if js_literal_expression | ||
| .as_js_null_literal_expression() | ||
| .is_some() | ||
| && !is_type_annotated | ||
| { | ||
| return Some(variable); | ||
| } | ||
| } |
There was a problem hiding this comment.
We can simplify here like:
match expression {
AnyJsExpression::AnyJsLiteralExpression(literal_expr) => {
if literal_expr.as_js_null_literal_expression().is_some()
&& !is_type_annotated
{
return Some(variable);
}
}
AnyJsExpression::JsArrayExpression(array_expr) => {
if array_expr.elements().into_iter().next().is_none() && !is_type_annotated
{
return Some(variable);
}
}
_ => continue
};
| }, | ||
| ) | ||
| .note(markup! { | ||
| "Variable's type may evolve, leading to "<Emphasis>"any"</Emphasis>". Use explicit type or initialization, e.g., 'let x: number;' or 'let x = 0;'." |
There was a problem hiding this comment.
IMO, currently we can reduce the examples. Because it may confuse users who should use string type annotation (or others). And we can describe what users should do here (or maybe good to add .note chain)
| "Variable's type may evolve, leading to "<Emphasis>"any"</Emphasis>". Use explicit type or initialization, e.g., 'let x: number;' or 'let x = 0;'." | |
| "Variable's type may evolve, leading to "<Emphasis>"any"</Emphasis>". Use explicit type or initialization. Specify an explicit type or initial value to avoid implicit type evolution." |
| /// ```ts,expect_diagnostic | ||
| /// let a = 'hello'; | ||
| /// const b = ['hello']; | ||
| /// const c = null; | ||
| /// ``` |
There was a problem hiding this comment.
I think this block is valid, so we can remove expect_diagnostic
| /// Disallow variables from evolving into `any` type through reassignments. | ||
| /// | ||
| /// In TypeScript, variables without explicit type annotations can evolve their types based on subsequent assignments. | ||
| /// This behavior can inadvertently lead to variables with an `any` type, weakening type safety. |
There was a problem hiding this comment.
IMHO, the word accidentally is used more frequently than inadvertently.
| /// This behavior can inadvertently lead to variables with an `any` type, weakening type safety. | |
| /// This behavior can accidentally lead to variables with an `any` type, weakening type safety. |
| rule_category!(), | ||
| variable.text_range(), | ||
| markup! { | ||
| "This variable's type is allowed to evolve implicitly, leading to potential "<Emphasis>"any"</Emphasis>" types. Specify an explicit type or initialization to avoid implicit type evolution." |
There was a problem hiding this comment.
I think this one sentence describes the error.
| "This variable's type is allowed to evolve implicitly, leading to potential "<Emphasis>"any"</Emphasis>" types. Specify an explicit type or initialization to avoid implicit type evolution." | |
| "This variable's type is allowed to evolve implicitly, leading to potential "<Emphasis>"any"</Emphasis>" types." |
|
@togami2864 @unvalley |
| Some( | ||
| RuleDiagnostic::new( | ||
| rule_category!(), | ||
| variable.text_range(), |
There was a problem hiding this comment.
You should use text_trimmed_range. If you check the snapshots, you will see that diagnostics show the variable and the space.
! This variable's type is allowed to evolve implicitly, leading to potential any types.
1 │ let a;
2 │ const b = [];
> 3 │ let c = null;
│ ^^
4 │
5 │ let someVar1;
text_trimmed_range will return the range without the trivia, while text_range will return the range with trivia
| /// ```ts,expect_diagnostic | ||
| /// let a; | ||
| /// const b = []; | ||
| /// let c = null; |
There was a problem hiding this comment.
I am not sure if you read the contribution guide, but when creating a snippet that should emit diagnostics, you should make sure that each snippet emits ONLY one diagnostic.
When adding invalid snippets in the ### Invalid section, you must use the expect_diagnostic code block property. We use this property to generate a diagnostic and attach it to the snippet. A snippet must emit only ONE diagnostic.
This means that you have to break down this snippet in THREE code blocks:
```ts,expect_diagnostic
let a;
```
```ts,expect_diagnostic
const b = [];
```There was a problem hiding this comment.
I sincerely apologize for overlooking that rule. 🙏 I will make the necessary corrections promptly.
There was a problem hiding this comment.
No problem! That's what a review is for :) to catch possible bugs!
|
I think there's something wrong with the snippets you used for the valid, check the rendered documentation page: https://deploy-preview-2112--biomejs.netlify.app/linter/rules/no-evolving-any/#_top It seems that the rule is triggered, which it should not. Could you check why? Try to run the command EDIT I was right: |
| rule_category!(), | ||
| variable.text_trimmed_range(), | ||
| markup! { | ||
| "This variable's type is allowed to evolve implicitly, leading to potential "<Emphasis>"any"</Emphasis>" types." |
There was a problem hiding this comment.
| "This variable's type is allowed to evolve implicitly, leading to potential "<Emphasis>"any"</Emphasis>" types." | |
| "This variable's type is not allowed to evolve implicitly, leading to potential "<Emphasis>"any"</Emphasis>" types." |
| }, | ||
| ) | ||
| .note(markup! { | ||
| "Variable's type may evolve, leading to "<Emphasis>"any"</Emphasis>". Use explicit type or initialization. Specify an explicit type or initial value to avoid implicit type evolution." |
There was a problem hiding this comment.
| "Variable's type may evolve, leading to "<Emphasis>"any"</Emphasis>". Use explicit type or initialization. Specify an explicit type or initial value to avoid implicit type evolution." | |
| "The variable's type may evolve, leading to "<Emphasis>"any"</Emphasis>". Use an explicit type or initialization. Specifying an explicit type or initial value to avoid implicit type evolution." |
| /// Disallow variables from evolving into `any` type through reassignments. | ||
| /// | ||
| /// In TypeScript, variables without explicit type annotations can evolve their types based on subsequent assignments. | ||
| /// This behavior can accidentally lead to variables with an `any` type, weakening type safety. | ||
| /// Just like the `any` type, evolved `any` types disable many type checking rules and should be avoided to maintain strong type safety. | ||
| /// This rule prevents such cases by ensuring variables do not evolve into `any` type, encouraging explicit type annotations and controlled type evolutions. |
There was a problem hiding this comment.
| /// Disallow variables from evolving into `any` type through reassignments. | |
| /// | |
| /// In TypeScript, variables without explicit type annotations can evolve their types based on subsequent assignments. | |
| /// This behavior can accidentally lead to variables with an `any` type, weakening type safety. | |
| /// Just like the `any` type, evolved `any` types disable many type checking rules and should be avoided to maintain strong type safety. | |
| /// This rule prevents such cases by ensuring variables do not evolve into `any` type, encouraging explicit type annotations and controlled type evolutions. | |
| /// Disallow variables from evolving into `any` type through reassignments. | |
| /// | |
| /// In TypeScript, variables without explicit type annotations can evolve their types based on subsequent assignments. | |
| /// This behaviour can accidentally lead to variables with an `any` type, weakening type safety. | |
| /// Just like the `any` type, evolved `any` types disable many type-checking rules and should be avoided to maintain strong type safety. | |
| /// This rule prevents such cases by ensuring variables do not evolve into `any` type, encouraging explicit type annotations and controlled type evolutions. |
|
@ematipico |
CHANGELOG.md
Outdated
There was a problem hiding this comment.
@fujiyamaorange Something formatter removes line breaks. Could you fix this?
Summary
#1883
Test Plan
Invalid
Valid