mirror of
https://github.com/danbulant/oxc
synced 2026-05-19 12:19:15 +00:00
feat(semantic): check for non-declared, non-abstract class accessors without bodies (#5460)
This should be causing more negative tests to fail than it actually is. This is because our typescript coverage tests use the "declarations" option to change the source type to `.d.ts`, which is incorrect. This setting enables `.d.ts` emits, it does not mean that input files should be treated as `.d.ts` themselves.
This commit is contained in:
parent
052e94c94e
commit
540714393a
4 changed files with 59 additions and 6 deletions
|
|
@ -1423,6 +1423,11 @@ impl MethodDefinitionKind {
|
|||
matches!(self, Self::Get)
|
||||
}
|
||||
|
||||
/// Returns `true` if this method is a getter or a setter.
|
||||
pub fn is_accessor(&self) -> bool {
|
||||
matches!(self, Self::Get | Self::Set)
|
||||
}
|
||||
|
||||
pub fn scope_flags(self) -> ScopeFlags {
|
||||
match self {
|
||||
Self::Constructor => ScopeFlags::Constructor | ScopeFlags::Function,
|
||||
|
|
|
|||
|
|
@ -287,8 +287,29 @@ fn parameter_property_only_in_constructor_impl(span: Span) -> OxcDiagnostic {
|
|||
.with_label(span)
|
||||
}
|
||||
|
||||
/// Getter or setter without a body. There is no corresponding TS error code,
|
||||
/// since in TSC this is a parse error.
|
||||
fn accessor_without_body(span: Span) -> OxcDiagnostic {
|
||||
OxcDiagnostic::error("Getters and setters must have an implementation.").with_label(span)
|
||||
}
|
||||
|
||||
pub fn check_method_definition<'a>(method: &MethodDefinition<'a>, ctx: &SemanticBuilder<'a>) {
|
||||
if method.r#type.is_abstract() {
|
||||
let is_abstract = method.r#type.is_abstract();
|
||||
let is_declare = ctx.class_table_builder.current_class_id.map_or(
|
||||
ctx.source_type.is_typescript_definition(),
|
||||
|id| {
|
||||
let ast_node_id = ctx.class_table_builder.classes.declarations[id];
|
||||
let AstKind::Class(class) = ctx.nodes.get_node(ast_node_id).kind() else {
|
||||
#[cfg(debug_assertions)]
|
||||
panic!("current_class_id is set, but does not point to a Class node.");
|
||||
#[cfg(not(debug_assertions))]
|
||||
return ctx.source_type.is_typescript_definition();
|
||||
};
|
||||
class.declare || ctx.source_type.is_typescript_definition()
|
||||
},
|
||||
);
|
||||
|
||||
if is_abstract {
|
||||
// constructors cannot be abstract, no matter what
|
||||
if method.kind.is_constructor() {
|
||||
ctx.error(illegal_abstract_modifier(method.key.span()));
|
||||
|
|
@ -313,16 +334,20 @@ pub fn check_method_definition<'a>(method: &MethodDefinition<'a>, ctx: &Semantic
|
|||
}
|
||||
}
|
||||
|
||||
let is_empty_body = method.value.r#type == FunctionType::TSEmptyBodyFunctionExpression;
|
||||
// Illegal to have `constructor(public foo);`
|
||||
if method.kind.is_constructor()
|
||||
&& method.value.r#type == FunctionType::TSEmptyBodyFunctionExpression
|
||||
{
|
||||
if method.kind.is_constructor() && is_empty_body {
|
||||
for param in &method.value.params.items {
|
||||
if param.accessibility.is_some() {
|
||||
ctx.error(parameter_property_only_in_constructor_impl(param.span));
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// Illegal to have `get foo();` or `set foo(a)`
|
||||
if method.kind.is_accessor() && is_empty_body && !is_abstract && !is_declare {
|
||||
ctx.error(accessor_without_body(method.key.span()));
|
||||
}
|
||||
}
|
||||
|
||||
pub fn check_property_definition<'a>(prop: &PropertyDefinition<'a>, ctx: &SemanticBuilder<'a>) {
|
||||
|
|
|
|||
|
|
@ -3,7 +3,7 @@ commit: 3bcfee23
|
|||
parser_babel Summary:
|
||||
AST Parsed : 2093/2101 (99.62%)
|
||||
Positive Passed: 2083/2101 (99.14%)
|
||||
Negative Passed: 1381/1493 (92.50%)
|
||||
Negative Passed: 1382/1493 (92.57%)
|
||||
Expect Syntax Error: tasks/coverage/babel/packages/babel-parser/test/fixtures/annex-b/enabled/3.1-sloppy-labeled-functions-if-body/input.js
|
||||
Expect Syntax Error: tasks/coverage/babel/packages/babel-parser/test/fixtures/core/categorized/invalid-fn-decl-labeled-inside-if/input.js
|
||||
Expect Syntax Error: tasks/coverage/babel/packages/babel-parser/test/fixtures/core/categorized/invalid-fn-decl-labeled-inside-loop/input.js
|
||||
|
|
@ -39,7 +39,6 @@ Expect Syntax Error: tasks/coverage/babel/packages/babel-parser/test/fixtures/ty
|
|||
Expect Syntax Error: tasks/coverage/babel/packages/babel-parser/test/fixtures/typescript/class/constructor-with-invalid-order-modifiers-3/input.ts
|
||||
Expect Syntax Error: tasks/coverage/babel/packages/babel-parser/test/fixtures/typescript/class/constructor-with-type-parameters/input.ts
|
||||
Expect Syntax Error: tasks/coverage/babel/packages/babel-parser/test/fixtures/typescript/class/constructor-with-type-parameters-babel-7/input.ts
|
||||
Expect Syntax Error: tasks/coverage/babel/packages/babel-parser/test/fixtures/typescript/class/declare-accessor/input.ts
|
||||
Expect Syntax Error: tasks/coverage/babel/packages/babel-parser/test/fixtures/typescript/class/declare-field-initializer/input.ts
|
||||
Expect Syntax Error: tasks/coverage/babel/packages/babel-parser/test/fixtures/typescript/class/declare-initializer/input.ts
|
||||
Expect Syntax Error: tasks/coverage/babel/packages/babel-parser/test/fixtures/typescript/class/declare-method/input.ts
|
||||
|
|
@ -10039,6 +10038,22 @@ Expect to Parse: tasks/coverage/babel/packages/babel-parser/test/fixtures/typesc
|
|||
8 │ abstract accessor f = 1;
|
||||
╰────
|
||||
|
||||
× Getters and setters must have an implementation.
|
||||
╭─[babel/packages/babel-parser/test/fixtures/typescript/class/declare-accessor/input.ts:2:15]
|
||||
1 │ class Foo {
|
||||
2 │ declare get foo()
|
||||
· ───
|
||||
3 │ declare set foo(v)
|
||||
╰────
|
||||
|
||||
× Getters and setters must have an implementation.
|
||||
╭─[babel/packages/babel-parser/test/fixtures/typescript/class/declare-accessor/input.ts:3:15]
|
||||
2 │ declare get foo()
|
||||
3 │ declare set foo(v)
|
||||
· ───
|
||||
4 │ }
|
||||
╰────
|
||||
|
||||
× Expected a semicolon or an implicit semicolon after a statement, but found none
|
||||
╭─[babel/packages/babel-parser/test/fixtures/typescript/class/declare-new-line-abstract/input.ts:1:8]
|
||||
1 │ declare abstract
|
||||
|
|
|
|||
|
|
@ -5021,6 +5021,14 @@ Expect to Parse: tasks/coverage/typescript/tests/cases/conformance/salsa/typeFro
|
|||
╰────
|
||||
help: Try insert a semicolon here
|
||||
|
||||
× Getters and setters must have an implementation.
|
||||
╭─[typescript/tests/cases/compiler/abstractPropertyNegative.ts:16:9]
|
||||
15 │ abstract notAllowed: string;
|
||||
16 │ get concreteWithNoBody(): string;
|
||||
· ──────────────────
|
||||
17 │ }
|
||||
╰────
|
||||
|
||||
× TS(1253): Abstract properties can only appear within an abstract class.
|
||||
╭─[typescript/tests/cases/compiler/abstractPropertyNegative.ts:15:14]
|
||||
14 │ readonly ro = "readonly please";
|
||||
|
|
|
|||
Loading…
Reference in a new issue