From 0c66b079d072bd776a0e8c644c26d1baa115be95 Mon Sep 17 00:00:00 2001 From: Dunqing Date: Mon, 5 Feb 2024 15:41:49 +0800 Subject: [PATCH] feat(semantic): report parameter related errors for set accessor --- crates/oxc_ast/src/ast/js.rs | 3 ++ crates/oxc_parser/src/diagnostics.rs | 10 ---- crates/oxc_parser/src/js/class.rs | 8 --- crates/oxc_parser/src/js/object.rs | 7 --- crates/oxc_semantic/src/checker/javascript.rs | 53 ++++++++++++++++++- tasks/coverage/parser_babel.snap | 9 +++- tasks/coverage/parser_typescript.snap | 28 ++++++++-- 7 files changed, 87 insertions(+), 31 deletions(-) diff --git a/crates/oxc_ast/src/ast/js.rs b/crates/oxc_ast/src/ast/js.rs index 3642b91336069b..21cc87440bb597 100644 --- a/crates/oxc_ast/src/ast/js.rs +++ b/crates/oxc_ast/src/ast/js.rs @@ -2084,6 +2084,9 @@ impl MethodDefinitionKind { pub fn is_method(&self) -> bool { matches!(self, Self::Method) } + pub fn is_set(&self) -> bool { + matches!(self, Self::Set) + } } #[derive(Debug, Clone, Hash)] diff --git a/crates/oxc_parser/src/diagnostics.rs b/crates/oxc_parser/src/diagnostics.rs index f06bec9dcc343f..9777c2c2115a89 100644 --- a/crates/oxc_parser/src/diagnostics.rs +++ b/crates/oxc_parser/src/diagnostics.rs @@ -252,16 +252,6 @@ pub struct IllegalNewline( #[diagnostic()] pub struct OptionalChainTaggedTemplate(#[label] pub Span); -#[derive(Debug, Error, Diagnostic)] -#[error("A 'get' accessor must not have any formal parameters.")] -#[diagnostic()] -pub struct GetterParameters(#[label] pub Span); - -#[derive(Debug, Error, Diagnostic)] -#[error("A 'set' accessor must have exactly one parameter.")] -#[diagnostic()] -pub struct SetterParameters(#[label] pub Span); - #[derive(Debug, Error, Diagnostic)] #[error("TS2681: A constructor cannot have a `this` parameter.")] #[diagnostic()] diff --git a/crates/oxc_parser/src/js/class.rs b/crates/oxc_parser/src/js/class.rs index d49a09c9710ff8..2f31daee5a4a58 100644 --- a/crates/oxc_parser/src/js/class.rs +++ b/crates/oxc_parser/src/js/class.rs @@ -361,14 +361,6 @@ impl<'a> Parser<'a> { let value = self.parse_method(r#async, generator)?; - if kind == MethodDefinitionKind::Get && value.params.parameters_count() != 0 { - self.error(diagnostics::GetterParameters(value.params.span)); - } - - if kind == MethodDefinitionKind::Set && value.params.parameters_count() != 1 { - self.error(diagnostics::SetterParameters(value.params.span)); - } - if kind == MethodDefinitionKind::Constructor { if let Some(this_param) = &value.this_param { // class Foo { constructor(this: number) {} } diff --git a/crates/oxc_parser/src/js/object.rs b/crates/oxc_parser/src/js/object.rs index cfe37a2a98982b..be9354fee1a627 100644 --- a/crates/oxc_parser/src/js/object.rs +++ b/crates/oxc_parser/src/js/object.rs @@ -217,9 +217,6 @@ impl<'a> Parser<'a> { self.expect(Kind::Get)?; let (key, computed) = self.parse_property_name()?; let method = self.parse_method(false, false)?; - if method.params.parameters_count() != 0 { - self.error(diagnostics::GetterParameters(method.params.span)); - } let value = self.ast.function_expression(method); Ok(self.ast.object_property( self.end_span(span), @@ -241,10 +238,6 @@ impl<'a> Parser<'a> { let (key, computed) = self.parse_property_name()?; let method = self.parse_method(false, false)?; - if method.params.parameters_count() != 1 { - self.error(diagnostics::SetterParameters(method.params.span)); - } - Ok(self.ast.object_property( self.end_span(span), PropertyKind::Set, diff --git a/crates/oxc_semantic/src/checker/javascript.rs b/crates/oxc_semantic/src/checker/javascript.rs index 9e49191bb719e8..e86719ca65836a 100644 --- a/crates/oxc_semantic/src/checker/javascript.rs +++ b/crates/oxc_semantic/src/checker/javascript.rs @@ -78,8 +78,9 @@ impl EarlyErrorJavaScript { } AstKind::Class(class) => check_class(class, node, ctx), - AstKind::Super(sup) => check_super(sup, node, ctx), + AstKind::MethodDefinition(method) => check_method_definition(method, ctx), AstKind::ObjectProperty(prop) => check_object_property(prop, ctx), + AstKind::Super(sup) => check_super(sup, node, ctx), AstKind::FormalParameters(params) => check_formal_parameters(params, node, ctx), AstKind::ArrayPattern(pat) => check_array_pattern(pat, ctx), @@ -791,6 +792,48 @@ fn check_class(class: &Class, node: &AstNode<'_>, ctx: &SemanticBuilder<'_>) { } } +fn check_setter(function: &Function<'_>, ctx: &SemanticBuilder<'_>) { + #[derive(Debug, Error, Diagnostic)] + #[error("A 'set' accessor must have exactly one parameter.")] + #[diagnostic()] + struct SetterWithParameters(#[label] Span); + + #[derive(Debug, Error, Diagnostic)] + #[error("A 'set' accessor cannot have rest parameter.")] + #[diagnostic()] + struct SetterWithRestParameter(#[label] Span); + + function.params.rest.as_ref().map_or_else( + || { + if function.params.parameters_count() != 1 { + ctx.error(SetterWithParameters(function.params.span)); + } + }, + |rest| { + ctx.error(SetterWithRestParameter(rest.span)); + }, + ); +} + +fn check_getter(function: &Function<'_>, ctx: &SemanticBuilder<'_>) { + #[derive(Debug, Error, Diagnostic)] + #[error("A 'get' accessor must not have any formal parameters.")] + #[diagnostic()] + pub struct GetterParameters(#[label] pub Span); + + if !function.params.items.is_empty() { + ctx.error(GetterParameters(function.params.span)); + } +} + +fn check_method_definition(method: &MethodDefinition<'_>, ctx: &SemanticBuilder<'_>) { + match method.kind { + MethodDefinitionKind::Set => check_setter(&method.value, ctx), + MethodDefinitionKind::Get => check_getter(&method.value, ctx), + _ => {} + } +} + fn check_super<'a>(sup: &Super, node: &AstNode<'a>, ctx: &SemanticBuilder<'a>) { #[derive(Debug, Error, Diagnostic)] #[error("'super' can only be referenced in a derived class.")] @@ -906,6 +949,14 @@ fn check_object_property(prop: &ObjectProperty, ctx: &SemanticBuilder<'_>) { if let Some(expr) = &prop.init { ctx.error(CoverInitializedName(expr.span())); } + + if let Expression::FunctionExpression(function) = &prop.value { + match prop.kind { + PropertyKind::Set => check_setter(function, ctx), + PropertyKind::Get => check_getter(function, ctx), + PropertyKind::Init => {} + } + } } #[derive(Debug, Error, Diagnostic)] diff --git a/tasks/coverage/parser_babel.snap b/tasks/coverage/parser_babel.snap index 0f83d32ad256e3..fdd24d57da179b 100644 --- a/tasks/coverage/parser_babel.snap +++ b/tasks/coverage/parser_babel.snap @@ -1,7 +1,7 @@ parser_babel Summary: AST Parsed : 2090/2096 (99.71%) Positive Passed: 2086/2096 (99.52%) -Negative Passed: 1360/1500 (90.67%) +Negative Passed: 1361/1500 (90.73%) Expect Syntax Error: "annex-b/disabled/1.1-html-comments-close/input.js" Expect Syntax Error: "annex-b/disabled/3.1-sloppy-labeled-functions/input.js" Expect Syntax Error: "annex-b/disabled/3.1-sloppy-labeled-functions-if-body/input.js" @@ -36,7 +36,6 @@ Expect Syntax Error: "es2020/dynamic-import-createImportExpression-false/invalid Expect Syntax Error: "esprima/es2015-arrow-function/invalid-param-strict-mode/input.js" Expect Syntax Error: "esprima/es2015-generator/generator-parameter-binding-property-reserved/input.js" Expect Syntax Error: "esprima/invalid-syntax/migrated_0101/input.js" -Expect Syntax Error: "esprima/rest-parameter/invalid-setter-rest/input.js" Expect Syntax Error: "typescript/arrow-function/async-rest-optional-parameter/input.ts" Expect Syntax Error: "typescript/cast/satisfies-const-error/input.ts" Expect Syntax Error: "typescript/cast/unparenthesized-assert-and-assign/input.ts" @@ -9461,6 +9460,12 @@ Expect to Parse: "typescript/types/const-type-parameters-babel-7/input.ts" · ────── ╰──── + × A 'set' accessor cannot have rest parameter. + ╭─[esprima/rest-parameter/invalid-setter-rest/input.js:1:13] + 1 │ x = { set f(...y) {} } + · ──── + ╰──── + × Invalid Unicode escape sequence ╭─[esprima/statement-expression/migrated_0002/input.js:1:2] 1 │ \\u0061 diff --git a/tasks/coverage/parser_typescript.snap b/tasks/coverage/parser_typescript.snap index 2247fc96c77724..c3b0d4eb901aec 100644 --- a/tasks/coverage/parser_typescript.snap +++ b/tasks/coverage/parser_typescript.snap @@ -1,7 +1,7 @@ parser_typescript Summary: AST Parsed : 5239/5243 (99.92%) Positive Passed: 5232/5243 (99.79%) -Negative Passed: 1038/4879 (21.27%) +Negative Passed: 1040/4879 (21.32%) Expect Syntax Error: "compiler/ClassDeclaration10.ts" Expect Syntax Error: "compiler/ClassDeclaration11.ts" Expect Syntax Error: "compiler/ClassDeclaration13.ts" @@ -33,7 +33,6 @@ Expect Syntax Error: "compiler/accessorAccidentalCallDiagnostic.ts" Expect Syntax Error: "compiler/accessorDeclarationEmitVisibilityErrors.ts" Expect Syntax Error: "compiler/accessorInferredReturnTypeErrorInReturnStatement.ts" Expect Syntax Error: "compiler/accessorWithInitializer.ts" -Expect Syntax Error: "compiler/accessorWithRestParam.ts" Expect Syntax Error: "compiler/accessorWithoutBody1.ts" Expect Syntax Error: "compiler/accessorWithoutBody2.ts" Expect Syntax Error: "compiler/accessorsInAmbientContext.ts" @@ -3173,7 +3172,6 @@ Expect Syntax Error: "conformance/parser/ecmascript5/MemberAccessorDeclarations/ Expect Syntax Error: "conformance/parser/ecmascript5/MemberAccessorDeclarations/parserMemberAccessorDeclaration11.ts" Expect Syntax Error: "conformance/parser/ecmascript5/MemberAccessorDeclarations/parserMemberAccessorDeclaration16.ts" Expect Syntax Error: "conformance/parser/ecmascript5/MemberAccessorDeclarations/parserMemberAccessorDeclaration17.ts" -Expect Syntax Error: "conformance/parser/ecmascript5/MemberAccessorDeclarations/parserMemberAccessorDeclaration18.ts" Expect Syntax Error: "conformance/parser/ecmascript5/MemberAccessorDeclarations/parserMemberAccessorDeclaration2.ts" Expect Syntax Error: "conformance/parser/ecmascript5/MemberAccessorDeclarations/parserMemberAccessorDeclaration3.ts" Expect Syntax Error: "conformance/parser/ecmascript5/MemberAccessorDeclarations/parserMemberAccessorDeclaration7.ts" @@ -4099,6 +4097,22 @@ Expect to Parse: "conformance/salsa/plainJSRedeclare3.ts" 6 │ } ╰──── + × A 'set' accessor cannot have rest parameter. + ╭─[compiler/accessorWithRestParam.ts:4:11] + 3 │ class C { + 4 │ set X(...v) { } + · ──── + 5 │ static set X(...v2) { } + ╰──── + + × A 'set' accessor cannot have rest parameter. + ╭─[compiler/accessorWithRestParam.ts:5:18] + 4 │ set X(...v) { } + 5 │ static set X(...v2) { } + · ───── + 6 │ } + ╰──── + × Unexpected token ╭─[compiler/aliasErrors.ts:13:12] 12 │ import m2 = no.mod; @@ -17151,6 +17165,14 @@ Expect to Parse: "conformance/salsa/plainJSRedeclare3.ts" 4 │ } ╰──── + × A 'set' accessor cannot have rest parameter. + ╭─[conformance/parser/ecmascript5/MemberAccessorDeclarations/parserMemberAccessorDeclaration18.ts:3:12] + 2 │ class C { + 3 │ set Foo(...a) { } + · ──── + 4 │ } + ╰──── + × Expected a semicolon or an implicit semicolon after a statement, but found none ╭─[conformance/parser/ecmascript5/MemberFunctionDeclarations/parserMemberFunctionDeclaration4.ts:2:11] 1 │ class C {