Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(semantic): report parameter related errors for setter/getter #2316

Conversation

Dunqing
Copy link
Member

@Dunqing Dunqing commented Feb 5, 2024

No description provided.

@github-actions github-actions bot added A-semantic Area - Semantic A-ast Area - AST labels Feb 5, 2024
Copy link

codspeed-hq bot commented Feb 5, 2024

CodSpeed Performance Report

Merging #2316 will not alter performance

Comparing 02-05-feat_semantic_report_parameter_related_errors_for_set_accessor (e2bcd8d) with main (9ca13d0)

Summary

✅ 27 untouched benchmarks

Copy link
Member

@Boshen Boshen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a javascript error with a missing case from

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) {} }
self.error(diagnostics::TSConstructorThisParameter(this_param.span));
}
if r#static {
self.error(diagnostics::StaticConstructor(key.span()));
}

I think it's easier to move these checks over to checker/javascript.js instead of keeping them in the parser?

Base automatically changed from 02-05-feat_semantic_report_type_parameter_list_cannot_be_empty to main February 5, 2024 08:05
@Dunqing
Copy link
Member Author

Dunqing commented Feb 5, 2024

This is a javascript error with a missing case from

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) {} }
self.error(diagnostics::TSConstructorThisParameter(this_param.span));
}
if r#static {
self.error(diagnostics::StaticConstructor(key.span()));
}

I think it's easier to move these checks over to checker/javascript.js instead of keeping them in the parser?

Yes, I will move the implementation to checker/javascript.js

@Dunqing Dunqing force-pushed the 02-05-feat_semantic_report_parameter_related_errors_for_set_accessor branch from 393e5eb to 0c66b07 Compare February 5, 2024 08:40
@github-actions github-actions bot added the A-parser Area - Parser label Feb 5, 2024
@Dunqing Dunqing changed the title feat(semantic): report parameter related errors for set accessor feat(semantic): report parameter related errors for setter/getter Feb 5, 2024
@Dunqing Dunqing force-pushed the 02-05-feat_semantic_report_parameter_related_errors_for_set_accessor branch from 0c66b07 to f5e2a24 Compare February 5, 2024 08:42
Copy link
Member

Boshen commented Feb 5, 2024

Merge activity

  • Feb 5, 4:38 AM EST: @Boshen started a stack merge that includes this pull request via Graphite.
  • Feb 5, 4:38 AM EST: @Boshen merged this pull request with Graphite.

@Boshen Boshen merged commit a3570d4 into main Feb 5, 2024
22 checks passed
@Boshen Boshen deleted the 02-05-feat_semantic_report_parameter_related_errors_for_set_accessor branch February 5, 2024 09:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-ast Area - AST A-parser Area - Parser A-semantic Area - Semantic
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants