-
-
Notifications
You must be signed in to change notification settings - Fork 504
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat(biome_js_analyzer): implement noImplicitAnyLet #395
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Original file line number | Diff line number | Diff line change | ||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
@@ -0,0 +1,90 @@ | ||||||||||||||||
use biome_analyze::{context::RuleContext, declare_rule, Ast, Rule, RuleDiagnostic}; | ||||||||||||||||
use biome_console::markup; | ||||||||||||||||
use biome_js_syntax::{JsFileSource, JsVariableDeclaration, JsVariableDeclarator}; | ||||||||||||||||
|
||||||||||||||||
declare_rule! { | ||||||||||||||||
/// Restrict use of implicit any type in Typescript. | ||||||||||||||||
/// | ||||||||||||||||
/// Typescript variable declaration without any `type` or `initialization` can cause issue later in the code. | ||||||||||||||||
/// | ||||||||||||||||
/// | ||||||||||||||||
Comment on lines
+8
to
+10
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I could add more details:
Suggested change
|
||||||||||||||||
/// | ||||||||||||||||
/// Source: https://www.typescriptlang.org/tsconfig#noImplicitAny | ||||||||||||||||
/// | ||||||||||||||||
/// ## Examples | ||||||||||||||||
/// | ||||||||||||||||
/// ### Invalid | ||||||||||||||||
/// | ||||||||||||||||
/// ```ts,expect_diagnostic | ||||||||||||||||
/// var a; | ||||||||||||||||
/// a = 2; | ||||||||||||||||
/// let b; | ||||||||||||||||
/// b = 1 | ||||||||||||||||
Comment on lines
+18
to
+22
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Each snippet in the invalid section must raise only one diagnostic, this means that you'd need to break down this snippet in four code blocks with The valid section doesn't have this restriction. |
||||||||||||||||
/// ``` | ||||||||||||||||
/// | ||||||||||||||||
/// ## Valid | ||||||||||||||||
/// | ||||||||||||||||
/// ```ts | ||||||||||||||||
/// var a = 1; | ||||||||||||||||
/// let a:number; | ||||||||||||||||
/// var b: number | ||||||||||||||||
/// var b =10; | ||||||||||||||||
/// ``` | ||||||||||||||||
/// | ||||||||||||||||
pub(crate) NoImplicitAnyLet { | ||||||||||||||||
version: "next", | ||||||||||||||||
name: "noImplicitAnyLet", | ||||||||||||||||
recommended: true, | ||||||||||||||||
} | ||||||||||||||||
} | ||||||||||||||||
|
||||||||||||||||
impl Rule for NoImplicitAnyLet { | ||||||||||||||||
type Query = Ast<JsVariableDeclaration>; | ||||||||||||||||
type State = JsVariableDeclarator; | ||||||||||||||||
type Signals = Option<Self::State>; | ||||||||||||||||
type Options = (); | ||||||||||||||||
|
||||||||||||||||
fn run(ctx: &RuleContext<Self>) -> Self::Signals { | ||||||||||||||||
let source_type = ctx.source_type::<JsFileSource>().language(); | ||||||||||||||||
let is_ts_source = source_type.is_typescript(); | ||||||||||||||||
let node = ctx.query(); | ||||||||||||||||
let is_declaration = source_type.is_definition_file(); | ||||||||||||||||
|
||||||||||||||||
if node.is_const() || is_declaration || !is_ts_source { | ||||||||||||||||
return None; | ||||||||||||||||
} | ||||||||||||||||
|
||||||||||||||||
for declarator in node.declarators() { | ||||||||||||||||
let variable = declarator.ok()?; | ||||||||||||||||
let is_initialized = variable.initializer().is_some(); | ||||||||||||||||
let is_type_annotated = variable.variable_annotation().is_some(); | ||||||||||||||||
if !is_initialized && !is_type_annotated { | ||||||||||||||||
return Some(variable); | ||||||||||||||||
} | ||||||||||||||||
} | ||||||||||||||||
|
||||||||||||||||
None | ||||||||||||||||
} | ||||||||||||||||
|
||||||||||||||||
fn diagnostic(_: &RuleContext<Self>, node: &Self::State) -> Option<RuleDiagnostic> { | ||||||||||||||||
let variable = node | ||||||||||||||||
.id() | ||||||||||||||||
.ok()? | ||||||||||||||||
.as_any_js_binding()? | ||||||||||||||||
.as_js_identifier_binding()? | ||||||||||||||||
.name_token() | ||||||||||||||||
.ok()?; | ||||||||||||||||
Some( | ||||||||||||||||
RuleDiagnostic::new( | ||||||||||||||||
rule_category!(), | ||||||||||||||||
variable.text_range(), | ||||||||||||||||
markup! { | ||||||||||||||||
"Variable " <Emphasis>{variable.text()}</Emphasis> " has implicitly " <Emphasis>"any"</Emphasis> " type" | ||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think we can remove the variable name.
Suggested change
|
||||||||||||||||
}, | ||||||||||||||||
) | ||||||||||||||||
.note(markup! { | ||||||||||||||||
"Declare type or initialize the variable with some value" | ||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We should also explain why the variable has implicitly the
Suggested change
|
||||||||||||||||
}), | ||||||||||||||||
) | ||||||||||||||||
} | ||||||||||||||||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,13 @@ | ||
let someVar1; | ||
someVar1 = '123'; | ||
someVar1 = 123; | ||
|
||
|
||
var someVar1; | ||
someVar1 = '123'; | ||
someVar1 = 123; | ||
|
||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Could you add some example with for loops, and multiple declarators? e.g. let x = 0, y, z = 0;
var x = 0, y, z = 0;
for(let a = 0, b; a < 5; a++) {} |
||
|
||
function ex() { | ||
let b; | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,68 @@ | ||
--- | ||
source: crates/biome_js_analyze/tests/spec_tests.rs | ||
expression: invalid.ts | ||
--- | ||
# Input | ||
```js | ||
let someVar1; | ||
someVar1 = '123'; | ||
someVar1 = 123; | ||
|
||
|
||
var someVar1; | ||
someVar1 = '123'; | ||
someVar1 = 123; | ||
|
||
|
||
function ex() { | ||
let b; | ||
} | ||
``` | ||
|
||
# Diagnostics | ||
``` | ||
invalid.ts:1:5 lint/nursery/noImplicitAnyLet ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ | ||
|
||
! Variable someVar1 has implicitly any type | ||
|
||
> 1 │ let someVar1; | ||
│ ^^^^^^^^ | ||
2 │ someVar1 = '123'; | ||
3 │ someVar1 = 123; | ||
|
||
i Declare type or initialize the variable with some value | ||
|
||
|
||
``` | ||
|
||
``` | ||
invalid.ts:6:5 lint/nursery/noImplicitAnyLet ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ | ||
|
||
! Variable someVar1 has implicitly any type | ||
|
||
> 6 │ var someVar1; | ||
│ ^^^^^^^^ | ||
7 │ someVar1 = '123'; | ||
8 │ someVar1 = 123; | ||
|
||
i Declare type or initialize the variable with some value | ||
|
||
|
||
``` | ||
|
||
``` | ||
invalid.ts:12:9 lint/nursery/noImplicitAnyLet ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ | ||
|
||
! Variable b has implicitly any type | ||
|
||
11 │ function ex() { | ||
> 12 │ let b; | ||
│ ^ | ||
13 │ } | ||
|
||
i Declare type or initialize the variable with some value | ||
|
||
|
||
``` | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,6 @@ | ||
/* should not generate diagnostics */ | ||
|
||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We should add e.g. const x = 0;
for(let y of xs) {}
using z = f(); |
||
let a: number; | ||
let b = 1 | ||
var c : string; | ||
var d = "abn" |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,16 @@ | ||
--- | ||
source: crates/biome_js_analyze/tests/spec_tests.rs | ||
expression: valid.ts | ||
--- | ||
# Input | ||
```js | ||
/* should not generate diagnostics */ | ||
|
||
let a: number; | ||
let b = 1 | ||
var c : string; | ||
var d = "abn" | ||
|
||
``` | ||
|
||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.