ESLint transition: update code so "@typescript-eslint/no-unused-vars" passes #192

Activity

1. cbraynor

   added

   good first issueGood for newcomersGood for newcomers

   on Sep 14, 2020
2. ericcornelissen commented on Nov 19, 2020

   

   ericcornelissen

   on Nov 19, 2020

   Contributor

   More actions

   Currently (aafb457) the output of npm run lint with the @typescript-eslint/no-unused-vars rule enabled is:

   ...\codeql-action\src\api-client.ts
     36:74  warning  '_' is defined but never used  @typescript-eslint/no-unused-vars
     37:68  warning  '_' is defined but never used  @typescript-eslint/no-unused-vars
   
   ...\codeql-action\src\config-utils.test.ts
     734:9  warning  '_queries' is defined but never used          @typescript-eslint/no-unused-vars
     735:9  warning  '_extraSearchPath' is defined but never used  @typescript-eslint/no-unused-vars
   

   I'm wondering if we want to remove them - which is possible, they're all trailing function arguments that can be removed safely without consequence - or if we want to configure the rule to allow unused variables if they either A) start with an underscore, or B) consist only of underscores. (This may desirable e.g. if you want to be explicit about the public function interface.) This can be archived by adding the following to the .eslintrc.json file:

   "rules": {
     "@typescript-eslint/no-unused-vars": ["error", {
       "argsIgnorePatternA": "_.*",
       "argsIgnorePatternB": "_+",
     }],
   }
3. cbraynor commented on Dec 23, 2020

   

   cbraynor

   on Dec 23, 2020

   ContributorAuthor

   More actions

   I'm sorry I dropped the ball on replying to this so long.

   I don't have a strong enough sense of what we're trying to achieve with these extra parameters to have a definitive answer for you, but perhaps @robertbrignull might?

   It seems like these parameters are superfluous and so could be removed fairly easily, but maybe there's value in keeping them in the codebase for documentation or something else?
4. robertbrignull commented on Jan 4, 2021

   

   robertbrignull

   on Jan 4, 2021

   Contributor

   More actions

   Regarding _queries and_extraSearchPath, I'm not sure what they are doing there either. Those are the args of the resolveQueries method but we aren't using them in that particular implementation. I think deleting those ares is fine. There are other definitions of resolveQueries in that file that don't have any args.

   I think the cases in api-client.ts have been removed since that comment was made, so that's good.

   So yeah enabling @typescript-eslint/no-unused-vars should be nice and easy now.
5. robertbrignull

   closed this as completedin #364on Jan 18, 2021
6. aofaof0907

   added a commit that references this issue on Jul 29, 2021

   Fix no shadow issues in api-client.ts

   ...

   c2273f7