diff --git a/crates/oxc_linter/src/rules/typescript/class_literal_property_style.rs b/crates/oxc_linter/src/rules/typescript/class_literal_property_style.rs index 778301160c..13d9277797 100644 --- a/crates/oxc_linter/src/rules/typescript/class_literal_property_style.rs +++ b/crates/oxc_linter/src/rules/typescript/class_literal_property_style.rs @@ -5,7 +5,7 @@ use oxc_ast::{ ast::{ ArrowFunctionExpression, AssignmentExpression, AssignmentTarget, Class, ClassBody, ClassElement, Expression, Function, MethodDefinitionKind, PropertyDefinition, PropertyKey, - Statement, + Statement, TSAccessibility, }, }; use oxc_ast_visit::VisitJs; @@ -125,7 +125,7 @@ declare_oxc_lint!( ClassLiteralPropertyStyle, typescript, style, - pending, + conditional_suggestion, config = ClassLiteralPropertyStyleOption, version = "1.47.0", short_description = "Enforces a consistent style for exposing literal values on classes.", @@ -163,7 +163,33 @@ fn check_fields_mode<'a>(class_body: &ClassBody<'a>, ctx: &LintContext<'a>) { && is_supported_literal(argument) && !has_duplicate_setter(class_body, method) { - ctx.diagnostic(prefer_field_style_diagnostic(method.key.span())); + if !method.decorators.is_empty() { + ctx.diagnostic(prefer_field_style_diagnostic(method.key.span())); + continue; + } + + ctx.diagnostic_with_suggestion( + prefer_field_style_diagnostic(method.key.span()), + |fixer| { + let mut replacement = + print_member_modifiers(method.accessibility, method.r#static, "readonly"); + push_member_key( + &mut replacement, + fixer.source_range(method.key.span()), + method.computed, + ); + if let Some(return_type) = &method.value.return_type { + replacement.push_str(fixer.source_range(return_type.span)); + } + replacement.push_str(" = "); + replacement.push_str(fixer.source_range(argument.span())); + replacement.push(';'); + + fixer + .replace(method.span, replacement) + .with_message("Replace the literals with readonly fields.") + }, + ); } } } @@ -183,7 +209,7 @@ fn check_getters_mode<'a>(class_body: &ClassBody<'a>, ctx: &LintContext<'a>) { for element in &class_body.body { if let ClassElement::PropertyDefinition(property) = element - && is_literal_readonly_property(property) + && let Some(value) = literal_readonly_property_value(property) { if let Some(name) = property.key.name() && excluded_properties.contains(&*name) @@ -191,11 +217,66 @@ fn check_getters_mode<'a>(class_body: &ClassBody<'a>, ctx: &LintContext<'a>) { continue; } - ctx.diagnostic(prefer_getter_style_diagnostic(property.key.span())); + if !property.decorators.is_empty() { + ctx.diagnostic(prefer_getter_style_diagnostic(property.key.span())); + continue; + } + + ctx.diagnostic_with_suggestion( + prefer_getter_style_diagnostic(property.key.span()), + |fixer| { + let mut replacement = + print_member_modifiers(property.accessibility, property.r#static, "get"); + push_member_key( + &mut replacement, + fixer.source_range(property.key.span()), + property.computed, + ); + replacement.push_str("()"); + if let Some(type_annotation) = &property.type_annotation { + replacement.push_str(fixer.source_range(type_annotation.span)); + } + replacement.push_str(" { return "); + replacement.push_str(fixer.source_range(value.span())); + replacement.push_str("; }"); + + fixer + .replace(property.span, replacement) + .with_message("Replace the literals with getters.") + }, + ); } } } +fn print_member_modifiers( + accessibility: Option, + is_static: bool, + final_modifier: &str, +) -> String { + let mut text = String::new(); + if let Some(accessibility) = accessibility { + text.push_str(accessibility.as_str()); + text.push(' '); + } + if is_static { + text.push_str("static "); + } + text.push_str(final_modifier); + text.push(' '); + text +} + +fn push_member_key(text: &mut String, key: &str, computed: bool) { + if computed { + text.push('['); + } + text.push_str(key); + if computed { + text.push(']'); + } +} + fn has_duplicate_setter<'a>( class_body: &ClassBody<'a>, getter: &oxc_ast::ast::MethodDefinition<'a>, @@ -211,11 +292,14 @@ fn has_duplicate_setter<'a>( }) } -fn is_literal_readonly_property(property: &PropertyDefinition<'_>) -> bool { - property.readonly - && !property.declare - && !property.r#override - && property.value.as_ref().is_some_and(is_supported_literal) +fn literal_readonly_property_value<'a, 'b>( + property: &'a PropertyDefinition<'b>, +) -> Option<&'a Expression<'b>> { + if !property.readonly || property.declare || property.r#override { + return None; + } + + property.value.as_ref().filter(|value| is_supported_literal(value)) } fn is_supported_literal(expression: &Expression<'_>) -> bool { @@ -657,6 +741,16 @@ fn test() { ", None, ), + ( + " + class Mx { + get foo(): string { + return 'x'; + } + } + ", + None, + ), ( " class Mx { @@ -693,6 +787,14 @@ fn test() { ", Some(serde_json::json!(["getters"])), ), + ( + " + class Mx { + readonly n: 1 | 2 = 1; + } + ", + Some(serde_json::json!(["getters"])), + ), ( " class Mx { @@ -826,6 +928,377 @@ fn test() { ), ]; + let fix = vec![ + ( + " + class Mx { + get p1() { + return 'hello world'; + } + } + ", + " + class Mx { + readonly p1 = 'hello world'; + } + ", + None, + ), + ( + " + class Mx { + get p1() { + return `hello world`; + } + } + ", + " + class Mx { + readonly p1 = `hello world`; + } + ", + None, + ), + ( + " + class Mx { + static get p1() { + return 'hello world'; + } + } + ", + " + class Mx { + static readonly p1 = 'hello world'; + } + ", + None, + ), + ( + " + class Mx { + public static get foo() { + return 1; + } + } + ", + " + class Mx { + public static readonly foo = 1; + } + ", + None, + ), + ( + " + class Mx { + get foo(): string { + return 'x'; + } + } + ", + " + class Mx { + readonly foo: string = 'x'; + } + ", + None, + ), + ( + " + class Mx { + public get [myValue]() { + return 'a literal value'; + } + } + ", + " + class Mx { + public readonly [myValue] = 'a literal value'; + } + ", + None, + ), + ( + " + class Mx { + public get [myValue]() { + return 12345n; + } + } + ", + " + class Mx { + public readonly [myValue] = 12345n; + } + ", + None, + ), + ( + " + class Mx { + public readonly [myValue] = 'a literal value'; + } + ", + " + class Mx { + public get [myValue]() { return 'a literal value'; } + } + ", + Some(serde_json::json!(["getters"])), + ), + ( + " + class Mx { + readonly p1 = 'hello world'; + } + ", + " + class Mx { + get p1() { return 'hello world'; } + } + ", + Some(serde_json::json!(["getters"])), + ), + ( + " + class Mx { + readonly n: 1 | 2 = 1; + } + ", + " + class Mx { + get n(): 1 | 2 { return 1; } + } + ", + Some(serde_json::json!(["getters"])), + ), + ( + " + class Mx { + readonly p1 = `hello world`; + } + ", + " + class Mx { + get p1() { return `hello world`; } + } + ", + Some(serde_json::json!(["getters"])), + ), + ( + " + class Mx { + static readonly p1 = 'hello world'; + } + ", + " + class Mx { + static get p1() { return 'hello world'; } + } + ", + Some(serde_json::json!(["getters"])), + ), + ( + " + class Mx { + protected get p1() { + return 'hello world'; + } + } + ", + " + class Mx { + protected readonly p1 = 'hello world'; + } + ", + Some(serde_json::json!(["fields"])), + ), + ( + " + class Mx { + protected readonly p1 = 'hello world'; + } + ", + " + class Mx { + protected get p1() { return 'hello world'; } + } + ", + Some(serde_json::json!(["getters"])), + ), + ( + " + class Mx { + public static get p1() { + return 'hello world'; + } + } + ", + " + class Mx { + public static readonly p1 = 'hello world'; + } + ", + None, + ), + ( + " + class Mx { + public static readonly p1 = 'hello world'; + } + ", + " + class Mx { + public static get p1() { return 'hello world'; } + } + ", + Some(serde_json::json!(["getters"])), + ), + ( + " + class Mx { + public get myValue() { + return gql` + { + user(id: 5) { + firstName + lastName + } + } + `; + } + } + ", + " + class Mx { + public readonly myValue = gql` + { + user(id: 5) { + firstName + lastName + } + } + `; + } + ", + None, + ), + ( + " + class Mx { + public readonly myValue = gql` + { + user(id: 5) { + firstName + lastName + } + } + `; + } + ", + " + class Mx { + public get myValue() { return gql` + { + user(id: 5) { + firstName + lastName + } + } + `; } + } + ", + Some(serde_json::json!(["getters"])), + ), + ( + " + class A { + private readonly foo: string = 'bar'; + constructor(foo: string) { + const bar = new (class { + private readonly foo: string = 'baz'; + constructor() { + this.foo = 'qux'; + } + })(); + } + } + ", + " + class A { + private get foo(): string { return 'bar'; } + constructor(foo: string) { + const bar = new (class { + private readonly foo: string = 'baz'; + constructor() { + this.foo = 'qux'; + } + })(); + } + } + ", + Some(serde_json::json!(["getters"])), + ), + ( + " + class A { + private readonly ['foo']: string = 'bar'; + constructor(foo: string) { + const bar = new (class { + private readonly foo: string = 'baz'; + constructor() {} + })(); + + if (bar) { + this.foo = 'baz'; + } + } + } + ", + " + class A { + private readonly ['foo']: string = 'bar'; + constructor(foo: string) { + const bar = new (class { + private get foo(): string { return 'baz'; } + constructor() {} + })(); + + if (bar) { + this.foo = 'baz'; + } + } + } + ", + Some(serde_json::json!(["getters"])), + ), + ( + " + class A { + private readonly foo: string = 'bar'; + constructor(foo: string) { + function func() { + this.foo = 'aa'; + } + } + } + ", + " + class A { + private get foo(): string { return 'bar'; } + constructor(foo: string) { + function func() { + this.foo = 'aa'; + } + } + } + ", + Some(serde_json::json!(["getters"])), + ), + ]; + Tester::new(ClassLiteralPropertyStyle::NAME, ClassLiteralPropertyStyle::PLUGIN, pass, fail) + .expect_fix(fix) .test_and_snapshot(); } diff --git a/crates/oxc_linter/src/snapshots/typescript_class_literal_property_style.snap b/crates/oxc_linter/src/snapshots/typescript_class_literal_property_style.snap index 57d8056643..e678578c31 100644 --- a/crates/oxc_linter/src/snapshots/typescript_class_literal_property_style.snap +++ b/crates/oxc_linter/src/snapshots/typescript_class_literal_property_style.snap @@ -38,6 +38,15 @@ source: crates/oxc_linter/src/tester.rs ╰──── help: Replace this getter with a readonly field initialized to the returned literal. + ⚠ typescript(class-literal-property-style): Literals should be exposed using readonly fields. + ╭─[class_literal_property_style.tsx:3:19] + 2 │ class Mx { + 3 │ get foo(): string { + · ─── + 4 │ return 'x'; + ╰──── + help: Replace this getter with a readonly field initialized to the returned literal. + ⚠ typescript(class-literal-property-style): Literals should be exposed using readonly fields. ╭─[class_literal_property_style.tsx:3:27] 2 │ class Mx { @@ -77,6 +86,15 @@ source: crates/oxc_linter/src/tester.rs ⚠ typescript(class-literal-property-style): Literals should be exposed using getters. ╭─[class_literal_property_style.tsx:3:24] 2 │ class Mx { + 3 │ readonly n: 1 | 2 = 1; + · ─ + 4 │ } + ╰──── + help: Replace this readonly literal field with a getter. + + ⚠ typescript(class-literal-property-style): Literals should be exposed using getters. + ╭─[class_literal_property_style.tsx:3:24] + 2 │ class Mx { 3 │ readonly p1 = `hello world`; · ── 4 │ } diff --git a/tasks/website_linter/src/rules/snapshots/docs_rule_pages.snap b/tasks/website_linter/src/rules/snapshots/docs_rule_pages.snap index 397c165b8d..adff822a1d 100644 --- a/tasks/website_linter/src/rules/snapshots/docs_rule_pages.snap +++ b/tasks/website_linter/src/rules/snapshots/docs_rule_pages.snap @@ -2099,7 +2099,7 @@ category: "Style" version: "1.47.0" default: false type_aware: false -fix: "pending" +fix: "conditional_suggestion" upstream: "https://typescript-eslint.io/rules/class-literal-property-style/" ---