Update code so "no-shadow" passes #310
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Remove the "no-shadow": "off" override and replace it by enabling "@typescript-eslint/no-shadow" in the "rules" section, following the typescript-eslint docs: https://github.com/typescript-eslint/typescript-eslint/blob/master/packages/eslint-plugin/docs/rules/no-shadow.md#how-to-use
Duplicate use of "_" placeholder argument name. This change may conflict with #192.
Two simple variable renames from "path" to "paths" since the types are arrays of strings (not just one string). One function definition inside a function moved outside that function to avoid shadowing the "options" argument.
Rename "queries" variable in test cases to "testQueries" to avoid shadowing it in a subsequent helper function call (4 times). Rename "path" twice in a hlper function to "validPath" and "invalidPath" to avoid shadowing "path". The new names are more explicit.
Rename throwaway variable "suite" to "found" when assigned from "find". Rename local variable "path" to "newPath" as it is a modification of the "originalPath" provided to `validateAndSanitisePath`. Rename instances of "path" to more explicit varients "ignorePath" and "includePath". Maybe "ignoredPath" and "includedPath" are better names?
Rename shadowing "uri" argument to the more explicit "artifactURI".
Rename various instances of "hash", shadowing the function with that name.
Rename one instance of "path" to avoid shadowing.
chrisgavin
approved these changes
Nov 20, 2020
Contributor
chrisgavin
left a comment
chrisgavin
left a comment
There was a problem hiding this comment.
Thanks! This looks good to me.
2 tasks
Merged
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Merge / deployment checklist
Closes #202
This updates the configuration so that
@typescript-eslint/no-shadowis enabled (instead of justno-shadow, following the@typescript-eslint/no-shadowdocumentation; otherwise some false positives will be outputted). I tried to split my commits per updated file so that I can motivate my changes. Below is an overview of this as well. Suggestions are certainly welcome, and the maintainers can feel free to change things as they see fit 🙂Changes
In
api-client.ts:In
codeql.ts:In
config-utils.test.ts:In
config-utils.ts:validateAndSanitisePath.In
fingerprints.test.ts:In
fingerprints.ts:In
upload-lib.ts: