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

🐛 noUnusedVariables incorrectly flags types, possible improvements #3669

Closed
1 task done
nstepien opened this issue Nov 11, 2022 · 0 comments · Fixed by #3860
Closed
1 task done

🐛 noUnusedVariables incorrectly flags types, possible improvements #3669

nstepien opened this issue Nov 11, 2022 · 0 comments · Fixed by #3860
Assignees
Labels
A-Linter Area: linter L-TypeScript Area: TypeScript support in Rome S-Bug: confirmed Status: report has been confirmed as a valid bug

Comments

@nstepien
Copy link
Contributor

nstepien commented Nov 11, 2022

Environment information

CLI:
  Version:              10.0.1
  Color support:        true

Platform:
  CPU Architecture:     x86_64
  OS:                   windows

Environment:
  ROME_LOG_DIR:         unset
  NO_COLOR:             unset
  TERM:                 unset

Rome Configuration:
  Status:               loaded
  Formatter disabled:   true
  Linter disabled:      false

Workspace:
  Open Documents:       0

Discovering running Rome servers...

Running Rome Server: ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━

i The client isn't connected to any server but rage discovered this running Rome server.

Server:
  Version:              10.0.1
  Name:                 rome_lsp
  CPU Architecture:     x86_64
  OS:                   windows

Workspace:
  Open Documents:       0

Other Active Server Workspaces:

Workspace:
  Open Documents:       2
  Client Name:          Visual Studio Code
  Client Version:       1.73.1

What happened?

interface Props {
  fn: ({ a, b }: { a: number; b: string }) => void;
}

try {
  fn();
} catch (error) {
  log();
}
.\test.tsx:2:10 lint/correctness/noUnusedVariables  FIXABLE  ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━

  ! This variable is unused.

    1 │ interface Props {
  > 2 │   fn: ({ a, b }: { a: number; b: string }) => void;
      │          ^
    3 │ }
    4 │ 

  i Unused variables usually are result of incomplete refactoring, typos and other source of bugs.

  i Suggested fix: If this is intentional, prepend a with an underscore.

     1  1 │   interface Props {
     2    │ - ··fn:·({·a,·b·}:·{·a:·number;·b:·string·})·=>·void;
        2 │ + ··fn:·({·_a,·b·}:·{·a:·number;·b:·string·})·=>·void;
     3  3 │   }
     4  4 │ 


.\test.tsx:2:13 lint/correctness/noUnusedVariables  FIXABLE  ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━

  ! This variable is unused.
  
    1 │ interface Props {
  > 2 │   fn: ({ a, b }: { a: number; b: string }) => void;
      │             ^
    3 │ }
    4 │ 

  i Unused variables usually are result of incomplete refactoring, typos and other source of bugs.

  i Suggested fix: If this is intentional, prepend b with an underscore.

     1  1 │   interface Props {
     2    │ - ··fn:·({·a,·b·}:·{·a:·number;·b:·string·})·=>·void;
        2 │ + ··fn:·({·a,·_b·}:·{·a:·number;·b:·string·})·=>·void;
     3  3 │   }
     4  4 │ 


.\test.tsx:7:10 lint/correctness/noUnusedVariables  FIXABLE  ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━

  ! This variable is unused.

    5 │ try {
    6 │   fn();
  > 7 │ } catch (error) {
      │          ^^^^^
    8 │   log();
    9 │ }

  i Unused variables usually are result of incomplete refactoring, typos and other source of bugs.

  i Suggested fix: If this is intentional, prepend error with an underscore.

     5  5 │   try {
     6  6 │     fn();
     7    │ - }·catch·(error)·{
        7 │ + }·catch·(_error)·{
     8  8 │     log();
     9  9 │   }

Expected result

In the interface, a and b are flagged as unused, the rule is suggesting to prefix them with _ which would give invalid types.

In catch (error), instead of prefixing error with an underscore, we can just omit it.

So Rome could give better suggestions so the code can end up like this:

interface Props {
  fn: (arg1: { a: number; b: string }) => void;
}

try {
  fn();
} catch {
  log();
}

Code of Conduct

  • I agree to follow Rome's Code of Conduct
@nstepien nstepien added the S-To triage Status: user report of a possible bug that needs to be triaged label Nov 11, 2022
@MichaReiser MichaReiser added S-Bug: confirmed Status: report has been confirmed as a valid bug A-Linter Area: linter L-TypeScript Area: TypeScript support in Rome and removed S-To triage Status: user report of a possible bug that needs to be triaged labels Nov 11, 2022
@ematipico ematipico added this to the 11.0.0 milestone Nov 14, 2022
@ematipico ematipico moved this to Todo in Rome 2022 Nov 16, 2022
Repository owner moved this from Todo to Done in Rome 2022 Dec 5, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A-Linter Area: linter L-TypeScript Area: TypeScript support in Rome S-Bug: confirmed Status: report has been confirmed as a valid bug
Projects
Status: Done
Development

Successfully merging a pull request may close this issue.

4 participants