Skip to content

Commit

Permalink
feat(biome_js_analyzer): noSubstr (#3125)
Browse files Browse the repository at this point in the history
  • Loading branch information
chansuke authored Jun 11, 2024
1 parent 8338f8e commit 4a7baaa
Show file tree
Hide file tree
Showing 13 changed files with 832 additions and 62 deletions.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

143 changes: 81 additions & 62 deletions crates/biome_configuration/src/linter/rules.rs

Large diffs are not rendered by default.

1 change: 1 addition & 0 deletions crates/biome_diagnostics_categories/src/categories.rs
Original file line number Diff line number Diff line change
Expand Up @@ -132,6 +132,7 @@ define_categories! {
"lint/nursery/noReactSpecificProps": "https://biomejs.dev/linter/rules/no-react-specific-props",
"lint/nursery/noRestrictedImports": "https://biomejs.dev/linter/rules/no-restricted-imports",
"lint/nursery/noShorthandPropertyOverrides": "https://biomejs.dev/linter/rules/no-shorthand-property-overrides",
"lint/nursery/noSubstr": "https://biomejs.dev/linter/rules/no-substr",
"lint/nursery/noTypeOnlyImportAttributes": "https://biomejs.dev/linter/rules/no-type-only-import-attributes",
"lint/nursery/noUndeclaredDependencies": "https://biomejs.dev/linter/rules/no-undeclared-dependencies",
"lint/nursery/noUnknownFunction": "https://biomejs.dev/linter/rules/no-unknown-function",
Expand Down
2 changes: 2 additions & 0 deletions crates/biome_js_analyze/src/lint/nursery.rs

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

206 changes: 206 additions & 0 deletions crates/biome_js_analyze/src/lint/nursery/no_substr.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,206 @@
use biome_analyze::{
context::RuleContext, declare_rule, ActionCategory, Ast, FixKind, Rule, RuleDiagnostic,
RuleSource,
};
use biome_console::markup;
use biome_js_factory::make;
use biome_js_syntax::{
AnyJsName, JsCallArguments, JsExpressionStatement, JsSyntaxToken, JsVariableStatement,
};
use biome_rowan::{declare_node_union, AstSeparatedList, BatchMutationExt, TextRange, TokenText};

use crate::JsRuleAction;

declare_rule! {
/// Enforce the use of `String.slice()` over `String.substr()` and `String.substring()`.
///
/// `String.slice()` is preferred over `String.substr()` and `String.substring()` because it is a more popular option with clearer behavior,
/// and it has a consistent counterpart in arrays.
///
/// Note that `String.substr`, `String.substring` and `String.slice` are not identical when arguments are passed.
/// For detailed differences, refer to the MDN documentation:
/// - [The difference between substring() and substr()](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/String/substring#the_difference_between_substring_and_substr)
/// - [Differences between substring() and slice()](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/String/substring#differences_between_substring_and_slice)
///
/// ## Examples
///
/// ### Invalid
///
/// ```js,expect_diagnostic
/// foo.substr();
/// ```
///
/// ```js,expect_diagnostic
/// foo.substring();
/// ```
///
/// ### Valid
///
/// ```js
/// foo.slice(beginIndex, endIndex);
/// ```
///
pub NoSubstr {
version: "next",
name: "noSubstr",
language: "js",
recommended: false,
sources: &[RuleSource::EslintUnicorn("prefer-string-slice")],
fix_kind: FixKind::Unsafe,
}
}

impl Rule for NoSubstr {
type Query = Ast<AnyJsStatement>;
type State = NoSubstrState;
type Signals = Option<Self::State>;
type Options = ();

fn run(ctx: &RuleContext<Self>) -> Self::Signals {
let node = ctx.query();
let value_token = node.value_token()?;

if matches!(value_token.text_trimmed(), "substr" | "substring") {
Some(NoSubstrState { value_token })
} else {
None
}
}

fn diagnostic(_: &RuleContext<Self>, state: &Self::State) -> Option<RuleDiagnostic> {
let diagnostic_message = markup! {
"Avoid using "{state.member_name().text()}" and consider using slice instead."
}
.to_owned();
let note_message = markup! {
<Emphasis>"slice"</Emphasis>" is more commonly used and has a less surprising behavior."
}
.to_owned();
let mdn_link =
markup! {
"See "<Hyperlink href="https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/String/substring#the_difference_between_substring_and_substr">"MDN web docs"</Hyperlink>" for more details."
}
.to_owned();
Some(
RuleDiagnostic::new(rule_category!(), state.span(), diagnostic_message)
.note(note_message)
.note(mdn_link),
)
}

fn action(ctx: &RuleContext<Self>, _: &Self::State) -> Option<JsRuleAction> {
let node = ctx.query();
let arguments = node.arguments()?;
let args = arguments.args();

if !args.is_empty() {
// If the function has arguments, we cannot replace it with slice() as it has different behavior.
// - https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/String/substring#differences_between_substring_and_slice
// - https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/String/substr#description
return None;
}

let mut mutation = ctx.root().begin();
let replaced_function = make::js_name(make::ident("slice"));
mutation.replace_element(node.member()?.into(), replaced_function.into());

Some(JsRuleAction::new(
ActionCategory::QuickFix,
ctx.metadata().applicability(),
markup! { "Use "<Emphasis>".slice()"</Emphasis>" instead." }.to_owned(),
mutation,
))
}
}

#[derive(Debug, Clone)]
pub struct NoSubstrState {
value_token: JsSyntaxToken,
}

impl NoSubstrState {
fn member_name(&self) -> TokenText {
self.value_token.token_text_trimmed()
}

fn span(&self) -> TextRange {
self.value_token.text_range()
}
}

// Helper union type to handle both JsExpressionStatement and JsVariableStatement.
// To handle arguments, we need to know the type of the statement.
declare_node_union! {
pub AnyJsStatement = JsExpressionStatement | JsVariableStatement
}

impl AnyJsStatement {
pub fn value_token(&self) -> Option<JsSyntaxToken> {
match self {
AnyJsStatement::JsExpressionStatement(node) => {
let callee = node
.expression()
.ok()?
.as_js_call_expression()?
.callee()
.ok()?;
callee
.as_js_static_member_expression()?
.member()
.ok()?
.value_token()
.ok()
}
AnyJsStatement::JsVariableStatement(node) => {
let declaration = node.declaration().ok()?;
let declarators = declaration.declarators();
declarators.into_iter().find_map(|declarator| {
let init = declarator.ok()?.initializer()?;
init.expression()
.ok()?
.as_js_static_member_expression()?
.member()
.ok()?
.value_token()
.ok()
})
}
}
}
pub fn member(&self) -> Option<AnyJsName> {
match self {
AnyJsStatement::JsExpressionStatement(node) => {
let callee = node
.expression()
.ok()?
.as_js_call_expression()?
.callee()
.ok()?;
callee.as_js_static_member_expression()?.member().ok()
}
AnyJsStatement::JsVariableStatement(node) => {
let declaration = node.declaration().ok()?;
let declarators = declaration.declarators();
declarators.into_iter().find_map(|declarator| {
let init = declarator.ok()?.initializer()?;
init.expression()
.ok()?
.as_js_static_member_expression()?
.member()
.ok()
})
}
}
}
pub fn arguments(&self) -> Option<JsCallArguments> {
match self {
AnyJsStatement::JsExpressionStatement(node) => node
.expression()
.ok()?
.as_js_call_expression()?
.arguments()
.ok(),
_ => None,
}
}
}
1 change: 1 addition & 0 deletions crates/biome_js_analyze/src/options.rs

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

20 changes: 20 additions & 0 deletions crates/biome_js_analyze/tests/specs/nursery/noSubstr/invalid.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
const f = foo.substr;
foo.substr()
foo?.substr()
foo.bar?.substring()
foo?.[0]?.substring()
foo.bar.substr?.()
foo.bar?.substring?.()
foo.bar?.baz?.substr()
foo.bar?.baz.substring()
foo.bar.baz?.substr()
"foo".substr()
"foo".substr(bar.length, Math.min(baz, 100)) // "foo".slice(bar.length, bar.length + Math.min(baz, 100))
"foo".substr(1, "abc".length) // "foo".slice(1, 1 + "abc".length)
"foo".substr("1", 2)
"foo".substring(length, 0) // "foo".slice(0, Math.max(0, length))
foo.substring(start) // foo.slice(Math.max(0, start))
foo.substring(start, end)
"foo".substring(1, 3)
// Extra arguments
foo.substring(1, 2, 3)
Loading

0 comments on commit 4a7baaa

Please sign in to comment.