quick-lint / quick-lint-js

quick-lint-js finds bugs in JavaScript programs
https://quick-lint-js.com
GNU General Public License v3.0
1.52k stars 192 forks source link

"false" positive errors: E0358: 'declare' should not be written inside a 'declare namespace' #1142

Closed strager closed 8 months ago

strager commented 8 months ago

quick-lint-js diagnoses this, perhaps incorrectly: https://github.com/getsentry/sentry/blob/5d08d3d6f625a7c3d9352edc0fbef641f2d9a768/static/app/types/react-router.d.ts#L31

skoch13 commented 8 months ago

Facing this within my project

strager commented 8 months ago

It looks like Sentry's tsconfig.json sets compilerOptions.skipLibCheck: true: https://github.com/getsentry/sentry/blob/5d08d3d6f625a7c3d9352edc0fbef641f2d9a768/config/tsconfig.base.json#L25

This suppresses errors in .d.ts files.

I think the right fix is to not have errors in the .d.ts files. I'm tempted to close this as a won't-fix because the code does have a bug.

@skoch13 Are you using compilerOptions.skipLibCheck: true?

skoch13 commented 8 months ago

@skoch13 Are you using compilerOptions.skipLibCheck: true?

@strager correct, we are using it

strager commented 8 months ago

I don't think this is actually a bug in quick-lint-js. TypeScript rejects the code too.

I think this bug is common enough that quick-lint-js's documentation should mention skipLibCheck.

skoch13 commented 8 months ago

I don't think this is actually a bug in quick-lint-js. TypeScript rejects the code too.

I think this bug is common enough that quick-lint-js's documentation should mention skipLibCheck.

I won’t argue with you on this, but I only observed it with quick-js-lint as without it my editor doesn’t show this as a warning

strager commented 8 months ago

I updated the docs to mention skipLibCheck: https://quick-lint-js.com/errors/E0358/