close
Skip to content

challenge(formatter): noInitializerWithDefinite rule#802

Merged
ematipico merged 5 commits intobiomejs:mainfrom
Yash-Singh1:challenge/typescript_definite
Nov 20, 2023
Merged

challenge(formatter): noInitializerWithDefinite rule#802
ematipico merged 5 commits intobiomejs:mainfrom
Yash-Singh1:challenge/typescript_definite

Conversation

@Yash-Singh1
Copy link
Copy Markdown
Contributor

Summary

This PR removes the bogus marking of TypeScript variable definitions with definite assertions that are initialized and instead moves it to a new lint rule noInitializerWithDefinite.

[yashsingh1]: Hi there, I was working on the typescript/definite/definite test case and traced the problem to definite declarations being marked as bogus by the parser here: https://github.com/biomejs/biome/blob/main/crates/biome_js_parser/src/syntax/stmt.rs#L1393. Would it be reasonable to not enter bogus mode as this can be considered as a compiler error instead of a syntax error?

[ematipico]: I think it's possible to change that, and create a syntax rule inside our analyzer

We already do that for a couple of cases https://github.com/biomejs/biome/blob/main/crates/biome_js_analyze/src/syntax/correctness/no_super_without_extends.rs

(https://discord.com/channels/1132231889290285117/1132602615570649110/1175767286968242266)

Test Plan

I added some test cases on biome_js_analyze and also have 100% Prettier Similarity for typescript/definite/definite.

@github-actions github-actions Bot added A-Linter Area: linter A-Parser Area: parser A-Formatter Area: formatter L-JavaScript Language: JavaScript and super languages A-Diagnostic Area: diagnostocis labels Nov 20, 2023
@TaKO8Ki
Copy link
Copy Markdown
Contributor

TaKO8Ki commented Nov 20, 2023

I guess you may need to run just ready or just codegen.

Copy link
Copy Markdown
Member

@ematipico ematipico left a comment

Choose a reason for hiding this comment

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

That's great!

Comment thread crates/biome_js_analyze/src/syntax/correctness/no_initializer_with_definite.rs Outdated
Yash-Singh1 and others added 2 commits November 20, 2023 08:21
Co-authored-by: unvalley <38400669+unvalley@users.noreply.github.com>
Copy link
Copy Markdown
Member

@Conaclos Conaclos left a comment

Choose a reason for hiding this comment

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

Thanks for your contribution! I left one comment.

Comment thread crates/biome_js_analyze/src/syntax/correctness/no_initializer_with_definite.rs Outdated
Comment thread crates/biome_js_analyze/src/syntax/correctness/no_initializer_with_definite.rs Outdated
Yash-Singh1 and others added 2 commits November 20, 2023 08:53
Co-authored-by: Victorien Elvinger <victorien@elvinger.fr>
@ematipico ematipico merged commit c616cb7 into biomejs:main Nov 20, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-Diagnostic Area: diagnostocis A-Formatter Area: formatter A-Linter Area: linter A-Parser Area: parser L-JavaScript Language: JavaScript and super languages

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants