Skip to content
This repository has been archived by the owner on Aug 31, 2023. It is now read-only.

feat(rome_js_analyze): no unused variables supporting ts overload #3135

Merged
merged 1 commit into from
Aug 30, 2022
Merged

feat(rome_js_analyze): no unused variables supporting ts overload #3135

merged 1 commit into from
Aug 30, 2022

Conversation

xunilrj
Copy link
Contributor

@xunilrj xunilrj commented Aug 30, 2022 •
edited by ematipico
Loading

Summary

Part of #2979

This PR allows Typescript function overloading to not be flagged as unused.

function used_overloaded(): number;
function used_overloaded(s: string): string;
function used_overloaded(s?: string) {
    return s;
}
used_overloaded();

Test Plan

> cargo test -p rome_js_analyze -- no_unused

@xunilrj xunilrj requested a review from leops as a code owner August 30, 2022 11:46
@xunilrj xunilrj requested a review from a team August 30, 2022 11:46
@xunilrj xunilrj temporarily deployed to aws August 30, 2022 11:46 Inactive
@github-actions
Copy link

Playground for commit adb6a5d

@cloudflare-workers-and-pages
Copy link

Deploying with  Cloudflare Pages  Cloudflare Pages

Latest commit: ec52f33
Status: ✅  Deploy successful!
Preview URL: https://81bd1f6c.tools-8rn.pages.dev
Branch Preview URL: https://feature-no-unused-ts-overloa.tools-8rn.pages.dev

View logs

@ematipico ematipico added this to the 0.9.0 milestone Aug 30, 2022
@ematipico ematipico added the A-Linter Area: linter label Aug 30, 2022
ematipico
ematipico reviewed Aug 30, 2022
View reviewed changes
crates/rome_js_analyze/tests/specs/js/noUnusedVariables/noUnusedVariables.ts.snap
warning[js/noUnusedVariables]: This function is unused.
┌─ noUnusedVariables.ts:12:10
│
12 │ function unused_overloaded(s?: string) {
Copy link
Contributor

Choose a reason for hiding this comment

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

I believe this diagnostic should not be here? If I understood the purpose of the PR correctly...

Copy link
Contributor Author

@xunilrj xunilrj Aug 30, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

This one is fine because all three overloaded functions are not used. And we flag only the one with a body.

@xunilrj xunilrj merged commit 7a42112 into main Aug 30, 2022
@xunilrj xunilrj deleted the feature/no-unused-ts-overload branch August 30, 2022 14:59
ematipico pushed a commit that referenced this pull request Sep 5, 2022
@xunilrj @ematipico
no unused variables supporting ts overload (#3135)
35115c8
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@ematipico ematipico ematipico left review comments

@leops leops leops approved these changes

Assignees
No one assigned
Labels
A-Linter Area: linter
Projects
Rome 2022
Status: Done
Milestone
0.9.0
Development

Successfully merging this pull request may close these issues.
3 participants
@xunilrj @ematipico @leops