Skip to content

chore: enable no-unsafe-assignment internally #3280

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged

Conversation

JoshuaKGoldberg
Copy link
Member

Continues #3278.

Most of the files that already contained numerous errors also disabled @typescript-eslint/no-explicit-any.

@typescript-eslint
Copy link
Contributor

Thanks for the PR, @JoshuaKGoldberg!

typescript-eslint is a 100% community driven project, and we are incredibly grateful that you are contributing to that community.

The core maintainers work on this in their personal time, so please understand that it may not be possible for them to review your work immediately.

Thanks again!


🙏 Please, if you or your company is finding typescript-eslint valuable, help us sustain the project by sponsoring it transparently on https://opencollective.com/typescript-eslint. As a thank you, your profile/company logo will be added to our main README which receives thousands of unique visitors per day.

@codecov
Copy link

codecov bot commented Apr 11, 2021

Codecov Report

Merging #3280 (3ec50ce) into master (cae4f4a) will not change coverage.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##           master    #3280   +/-   ##
=======================================
  Coverage   92.68%   92.68%           
=======================================
  Files         318      318           
  Lines       11100    11100           
  Branches     3152     3152           
=======================================
  Hits        10288    10288           
  Misses        361      361           
  Partials      451      451           
Flag Coverage Δ
unittest 92.68% <100.00%> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
...lugin/src/rules/indent-new-do-not-use/TokenInfo.ts 100.00% <ø> (ø)
packages/eslint-plugin/src/rules/indent.ts 90.47% <ø> (ø)
...kages/eslint-plugin/src/rules/naming-convention.ts 81.18% <ø> (ø)
...ackages/eslint-plugin/src/rules/no-extra-parens.ts 90.27% <ø> (ø)
...ages/eslint-plugin/src/rules/unified-signatures.ts 92.68% <ø> (ø)
...ils/src/ast-utils/eslint-utils/ReferenceTracker.ts 100.00% <ø> (ø)
...es/experimental-utils/src/ts-eslint-scope/index.ts 100.00% <ø> (ø)
packages/scope-manager/src/scope/GlobalScope.ts 94.73% <ø> (ø)
packages/typescript-estree/src/convert.ts 98.24% <ø> (ø)
packages/eslint-plugin-tslint/src/rules/config.ts 97.36% <100.00%> (ø)
... and 3 more

Comment on lines +7 to +12
/* eslint-disable @typescript-eslint/no-unsafe-assignment */
const ReferenceTrackerREAD: unique symbol = eslintUtils.ReferenceTracker.READ;
const ReferenceTrackerCALL: unique symbol = eslintUtils.ReferenceTracker.CALL;
const ReferenceTrackerCONSTRUCT: unique symbol =
eslintUtils.ReferenceTracker.CONSTRUCT;
/* eslint-enable @typescript-eslint/no-unsafe-assignment */
Copy link
Member

Choose a reason for hiding this comment

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

Copy link
Member Author

Choose a reason for hiding this comment

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

I went down a bit of a rabbit hole on this one... In theory we could turn them into their own unique symbols, and use typeof ... for the types here.

Unfortunately that then means that the eslint-utils/ReferenceTracker.d.ts output file now has an import from eslint-utils, which causes compile complaints:

Error: @typescript-eslint/eslint-plugin-internal: ../experimental-utils/dist/ast-utils/eslint-utils/ReferenceTracker.d.ts(1,30): error TS7016: Could not find a declaration file for module 'eslint-utils'. '/home/runner/work/typescript-eslint/typescript-eslint/node_modules/eslint-utils/index.js' implicitly has an 'any' type.

At that point I gave up.

@@ -5,4 +5,5 @@ export {
} from '@typescript-eslint/typescript-estree';

// note - cannot migrate this to an import statement because it will make TSC copy the package.json to the dist folder
// eslint-disable-next-line @typescript-eslint/no-unsafe-assignment
export const version: string = require('../package.json').version;
Copy link
Member

Choose a reason for hiding this comment

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

perhaps we should add a global type declaration to the project.
Something like

declare module '../package.json' {
  export const version = string;
}

would that fix this problem?

Copy link
Member Author

Choose a reason for hiding this comment

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

I don't think so 😕 , in @types/node/globals.d.ts NodeJS.Require is typed as returning any from its (id: string) function signature.

interface Require {
  (id: string): any;

@bradzacher bradzacher added awaiting response Issues waiting for a reply from the OP or another party repo maintenance things to do with maintenance of the repo, and not with code/docs labels Apr 12, 2021
@JoshuaKGoldberg
Copy link
Member Author

I'm waiting to update this and continue internal linting work until #3281 is in, btw.

@bradzacher bradzacher removed the awaiting response Issues waiting for a reply from the OP or another party label Apr 19, 2021
@bradzacher bradzacher added the awaiting response Issues waiting for a reply from the OP or another party label May 15, 2021
const ReferenceTrackerREAD: unique symbol = eslintUtils.ReferenceTracker.READ;
const ReferenceTrackerCALL: unique symbol = eslintUtils.ReferenceTracker.CALL;
const ReferenceTrackerCONSTRUCT: unique symbol =
// TODO: FILE ISSUE(S) SOMEWHERE
Copy link
Member Author

Choose a reason for hiding this comment

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

@bradzacher bradzacher removed the awaiting response Issues waiting for a reply from the OP or another party label May 24, 2021
Copy link
Member

@bradzacher bradzacher left a comment

Choose a reason for hiding this comment

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

lgtm - thanks for turning this on!

@bradzacher bradzacher merged commit 9524424 into typescript-eslint:master May 28, 2021
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jun 28, 2021
@JoshuaKGoldberg JoshuaKGoldberg deleted the no-unsafe-assignment branch November 17, 2021 07:45
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
repo maintenance things to do with maintenance of the repo, and not with code/docs
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants