Skip to content

Commit

Permalink
feat(semantic): report parameter related errors for set accessor
Browse files Browse the repository at this point in the history
  • Loading branch information
Dunqing committed Feb 5, 2024
1 parent a66e71a commit 0c66b07
Show file tree
Hide file tree
Showing 7 changed files with 87 additions and 31 deletions.
3 changes: 3 additions & 0 deletions crates/oxc_ast/src/ast/js.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)]
Expand Down
10 changes: 0 additions & 10 deletions crates/oxc_parser/src/diagnostics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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()]
Expand Down
8 changes: 0 additions & 8 deletions crates/oxc_parser/src/js/class.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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) {} }
Expand Down
7 changes: 0 additions & 7 deletions crates/oxc_parser/src/js/object.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand All @@ -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,
Expand Down
53 changes: 52 additions & 1 deletion crates/oxc_semantic/src/checker/javascript.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand Down Expand Up @@ -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.")]
Expand Down Expand Up @@ -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)]
Expand Down
9 changes: 7 additions & 2 deletions tasks/coverage/parser_babel.snap
Original file line number Diff line number Diff line change
@@ -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"
Expand Down Expand Up @@ -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"
Expand Down Expand Up @@ -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]
1x = { set f(...y) {} }
· ────
╰────
× Invalid Unicode escape sequence
╭─[esprima/statement-expression/migrated_0002/input.js:1:2]
1 │ \\u0061
Expand Down
28 changes: 25 additions & 3 deletions tasks/coverage/parser_typescript.snap
Original file line number Diff line number Diff line change
@@ -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"
Expand Down Expand Up @@ -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"
Expand Down Expand Up @@ -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"
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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 {
Expand Down

0 comments on commit 0c66b07

Please sign in to comment.