microsoft/pyright
1
1
Fork 0
mirror of https://github.com/microsoft/pyright.git synced 2024-08-16 11:20:22 +03:00
Code Issues Projects Releases Wiki Activity

Added new diagnostic check reportUnnecessaryTypeIgnoreComment that emits a diagnostic when a # type: ignore comment has no effect.

Browse Source
This commit is contained in:
Eric Traut 2022-01-09 11:16:22 -07:00
parent 2258c9a228
commit f9f4c34b5e
13 changed files with 167 additions and 30 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

3
docs/configuration.md
View File

@ -160,6 +160,8 @@ The following settings control pyright’s diagnostic output (warnings or errors
**reportUnusedCoroutine** [boolean or string, optional]: Generate or suppress diagnostics for call statements whose return value is not used in any way and is a Coroutine. This identifies a common error where an `await` keyword is mistakenly omitted. The default value for this setting is 'error'.
**reportUnnecessaryTypeIgnoreComment** [boolean or string, optional]: Generate or suppress diagnostics for a '# type: ignore' comment that would have no effect if removed.
## Execution Environment Options
Pyright allows multiple “execution environments” to be defined for different portions of your source tree. For example, a subtree may be designed to run with different import search paths or a different version of the python interpreter than the rest of the source base.
@ -327,6 +329,7 @@ The following table lists the default severity levels for each diagnostic rule w
| reportUnsupportedDunderAll | "none" | "warning" | "error" |
| reportUnusedCallResult | "none" | "none" | "none" |
| reportUnusedCoroutine | "none" | "error" | "error" |
| reportUnnecessaryTypeIgnoreComment | "none" | "none" | "none" |

76
packages/pyright-internal/src/analyzer/sourceFile.ts
View File

@ -28,6 +28,7 @@ import { TextEditAction } from '../common/editAction';
import { FileSystem } from '../common/fileSystem';
import { LogTracker } from '../common/logTracker';
import { getFileName, normalizeSlashes, stripFileExtension } from '../common/pathUtils';
import { convertOffsetsToRange } from '../common/positionUtils';
import * as StringUtils from '../common/stringUtils';
import { DocumentRange, getEmptyRange, Position, TextRange } from '../common/textRange';
import { TextRangeCollection } from '../common/textRangeCollection';
@ -147,8 +148,8 @@ export class SourceFile {
private _parseDiagnostics: Diagnostic[] = [];
private _bindDiagnostics: Diagnostic[] = [];
private _checkerDiagnostics: Diagnostic[] = [];
private _typeIgnoreLines: { [line: number]: boolean } = {};
private _typeIgnoreAll = false;
private _typeIgnoreLines = new Map<number, TextRange>();
private _typeIgnoreAll: TextRange | undefined;
// Settings that control which diagnostics should be output.
private _diagnosticRuleSet = getBasicDiagnosticRuleSet();
@ -254,16 +255,18 @@ export class SourceFile {
includeWarningsAndErrors = false;
}
let diagList: Diagnostic[] = [];
diagList = diagList.concat(this._parseDiagnostics, this._bindDiagnostics, this._checkerDiagnostics);
let diagList = [...this._parseDiagnostics, ...this._bindDiagnostics, ...this._checkerDiagnostics];
const prefilteredDiagList = diagList;
const typeIgnoreLinesClone = new Map(this._typeIgnoreLines);
// Filter the diagnostics based on "type: ignore" lines.
if (options.diagnosticRuleSet.enableTypeIgnoreComments) {
if (Object.keys(this._typeIgnoreLines).length > 0) {
if (this._typeIgnoreLines.size > 0) {
diagList = diagList.filter((d) => {
if (d.category !== DiagnosticCategory.UnusedCode && d.category !== DiagnosticCategory.Deprecated) {
for (let line = d.range.start.line; line <= d.range.end.line; line++) {
if (this._typeIgnoreLines[line]) {
if (this._typeIgnoreLines.has(line)) {
typeIgnoreLinesClone.delete(line);
return false;
}
}
@ -274,6 +277,49 @@ export class SourceFile {
}
}
const unnecessaryTypeIgnoreDiags: Diagnostic[] = [];
if (options.diagnosticRuleSet.reportUnnecessaryTypeIgnoreComment !== 'none') {
const diagCategory = convertLevelToCategory(options.diagnosticRuleSet.reportUnnecessaryTypeIgnoreComment);
const prefilteredErrorList = prefilteredDiagList.filter(
(diag) =>
diag.category === DiagnosticCategory.Error ||
diag.category === DiagnosticCategory.Warning ||
diag.category === DiagnosticCategory.Information
);
if (prefilteredErrorList.length === 0 && this._typeIgnoreAll !== undefined) {
unnecessaryTypeIgnoreDiags.push(
new Diagnostic(
diagCategory,
Localizer.Diagnostic.unnecessaryTypeIgnore(),
convertOffsetsToRange(
this._typeIgnoreAll.start,
this._typeIgnoreAll.start + this._typeIgnoreAll.length,
this._parseResults!.tokenizerOutput.lines!
)
)
);
}
typeIgnoreLinesClone.forEach((textRange) => {
if (this._parseResults?.tokenizerOutput.lines) {
unnecessaryTypeIgnoreDiags.push(
new Diagnostic(
diagCategory,
Localizer.Diagnostic.unnecessaryTypeIgnore(),
convertOffsetsToRange(
textRange.start,
textRange.start + textRange.length,
this._parseResults!.tokenizerOutput.lines!
)
)
);
}
});
}
if (options.diagnosticRuleSet.reportImportCycles !== 'none' && this._circularDependencies.length > 0) {
const category = convertLevelToCategory(options.diagnosticRuleSet.reportImportCycles);
@ -309,13 +355,21 @@ export class SourceFile {
}
// If there is a "type: ignore" comment at the top of the file, clear
// the diagnostic list.
// the diagnostic list of all error, warning, and information diagnostics.
if (options.diagnosticRuleSet.enableTypeIgnoreComments) {
if (this._typeIgnoreAll) {
diagList = [];
if (this._typeIgnoreAll !== undefined) {
diagList = diagList.filter(
(diag) =>
diag.category !== DiagnosticCategory.Error &&
diag.category !== DiagnosticCategory.Warning &&
diag.category !== DiagnosticCategory.Information
);
}
}
// Now add in the "unnecessary type ignore" diagnostics.
diagList.push(...unnecessaryTypeIgnoreDiags);
// If we're not returning any diagnostics, filter out all of
// the errors and warnings, leaving only the unreachable code
// and deprecated diagnostics.
@ -643,8 +697,8 @@ export class SourceFile {
tokenizerOutput: {
tokens: new TextRangeCollection<Token>([]),
lines: new TextRangeCollection<TextRange>([]),
typeIgnoreAll: false,
typeIgnoreLines: {},
typeIgnoreAll: undefined,
typeIgnoreLines: new Map<number, TextRange>(),
predominantEndOfLineSequence: '\n',
predominantTabSequence: ' ',
predominantSingleQuoteCharacter: "'",

15
packages/pyright-internal/src/common/configOptions.ts
View File

@ -271,6 +271,10 @@ export interface DiagnosticRuleSet {
// Report cases where a call expression's return result is Coroutine
// and is not used in any way.
reportUnusedCoroutine: DiagnosticLevel;
// Report cases where the removal of a "# type: ignore" comment would
// have no effect.
reportUnnecessaryTypeIgnoreComment: DiagnosticLevel;
}
export function cloneDiagnosticRuleSet(diagSettings: DiagnosticRuleSet): DiagnosticRuleSet {
@ -348,6 +352,7 @@ export function getDiagLevelDiagnosticRules() {
DiagnosticRule.reportUnsupportedDunderAll,
DiagnosticRule.reportUnusedCallResult,
DiagnosticRule.reportUnusedCoroutine,
DiagnosticRule.reportUnnecessaryTypeIgnoreComment,
];
}
@ -423,6 +428,7 @@ export function getOffDiagnosticRuleSet(): DiagnosticRuleSet {
reportUnsupportedDunderAll: 'none',
reportUnusedCallResult: 'none',
reportUnusedCoroutine: 'none',
reportUnnecessaryTypeIgnoreComment: 'none',
};
return diagSettings;
@ -494,6 +500,7 @@ export function getBasicDiagnosticRuleSet(): DiagnosticRuleSet {
reportUnsupportedDunderAll: 'warning',
reportUnusedCallResult: 'none',
reportUnusedCoroutine: 'error',
reportUnnecessaryTypeIgnoreComment: 'none',
};
return diagSettings;
@ -565,6 +572,7 @@ export function getStrictDiagnosticRuleSet(): DiagnosticRuleSet {
reportUnsupportedDunderAll: 'error',
reportUnusedCallResult: 'none',
reportUnusedCoroutine: 'error',
reportUnnecessaryTypeIgnoreComment: 'none',
};
return diagSettings;
@ -1288,6 +1296,13 @@ export class ConfigOptions {
DiagnosticRule.reportUnusedCoroutine,
defaultSettings.reportUnusedCoroutine
),
// Read the "reportUnusedCoroutine" entry.
reportUnnecessaryTypeIgnoreComment: this._convertDiagnosticLevel(
configObj.reportUnnecessaryTypeIgnoreComment,
DiagnosticRule.reportUnnecessaryTypeIgnoreComment,
defaultSettings.reportUnnecessaryTypeIgnoreComment
),
};
// Read the "venvPath".

1
packages/pyright-internal/src/common/diagnosticRules.ts
View File

@ -71,4 +71,5 @@ export enum DiagnosticRule {
reportUnsupportedDunderAll = 'reportUnsupportedDunderAll',
reportUnusedCallResult = 'reportUnusedCallResult',
reportUnusedCoroutine = 'reportUnusedCoroutine',
reportUnnecessaryTypeIgnoreComment = 'reportUnnecessaryTypeIgnoreComment',
}

1
packages/pyright-internal/src/localization/localize.ts
View File

@ -848,6 +848,7 @@ export namespace Localizer {
new ParameterizedString<{ testType: string; classType: string }>(
getRawString('Diagnostic.unnecessaryIsSubclassAlways')
);
export const unnecessaryTypeIgnore = () => getRawString('Diagnostic.unnecessaryTypeIgnore');
export const unpackArgCount = () => getRawString('Diagnostic.unpackArgCount');
export const unpackedArgInTypeArgument = () => getRawString('Diagnostic.unpackedArgInTypeArgument');
export const unpackedArgWithVariadicParam = () => getRawString('Diagnostic.unpackedArgWithVariadicParam');

1
packages/pyright-internal/src/localization/package.nls.en-us.json
View File

@ -431,6 +431,7 @@
"unnecessaryCast": "Unnecessary \"cast\" call; type is already \"{type}\"",
"unnecessaryIsInstanceAlways": "Unnecessary isinstance call; \"{testType}\" is always an instance of \"{classType}\"",
"unnecessaryIsSubclassAlways": "Unnecessary issubclass call; \"{testType}\" is always a subclass of \"{classType}\"",
"unnecessaryTypeIgnore": "Unnecessary '# type ignore' comment",
"unpackArgCount": "Expected a single type argument after \"Unpack\"",
"unpackedArgInTypeArgument": "Unpacked arguments cannot be used in type argument lists",
"unpackedArgWithVariadicParam": "Unpacked argument cannot be used for TupleTypeVar parameter",

20
packages/pyright-internal/src/parser/tokenizer.ts
View File

@ -145,10 +145,10 @@ export interface TokenizerOutput {
lines: TextRangeCollection<TextRange>;
// Map of all line numbers that end in a "type: ignore" comment.
typeIgnoreLines: { [line: number]: boolean };
typeIgnoreLines: Map<number, TextRange>;
// Program starts with a "type: ignore" comment.
typeIgnoreAll: boolean;
typeIgnoreAll: TextRange | undefined;
// Line-end sequence ('/n', '/r', or '/r/n').
predominantEndOfLineSequence: string;
@ -180,8 +180,8 @@ export class Tokenizer {
private _parenDepth = 0;
private _lineRanges: TextRange[] = [];
private _indentAmounts: IndentInfo[] = [];
private _typeIgnoreAll = false;
private _typeIgnoreLines: { [line: number]: boolean } = {};
private _typeIgnoreAll: TextRange | undefined;
private _typeIgnoreLines = new Map<number, TextRange>();
private _comments: Comment[] | undefined;
// Total times CR, CR/LF, and LF are used to terminate
@ -1004,11 +1004,17 @@ export class Tokenizer {
// ignore comments of the form ignore[errorCode, ...]. We'll treat
// these as regular ignore statements (as though no errorCodes were
// included).
if (value.match(/^\s*type:\s*ignore(\s|\[|$)/)) {
const regexMatch = value.match(/^\s*type:\s*ignore(\s|\[|$)/);
if (regexMatch) {
const textRange: TextRange = { start, length: regexMatch[0].length };
if (regexMatch[0].endsWith('[')) {
textRange.length--;
}
if (this._tokens.findIndex((t) => t.type !== TokenType.NewLine && t && t.type !== TokenType.Indent) < 0) {
this._typeIgnoreAll = true;
this._typeIgnoreAll = textRange;
} else {
this._typeIgnoreLines[this._lineRanges.length] = true;
this._typeIgnoreLines.set(this._lineRanges.length, textRange);
}
}

22
packages/pyright-internal/src/tests/checker.test.ts
View File

@ -264,6 +264,28 @@ test('TypeIgnore3', () => {
TestUtils.validateResults(analysisResults, 4);
});
test('TypeIgnore4', () => {
const configOptions = new ConfigOptions('.');
let analysisResults = TestUtils.typeAnalyzeSampleFiles(['typeIgnore4.py'], configOptions);
TestUtils.validateResults(analysisResults, 0);
configOptions.diagnosticRuleSet.reportUnnecessaryTypeIgnoreComment = 'error';
analysisResults = TestUtils.typeAnalyzeSampleFiles(['typeIgnore4.py'], configOptions);
TestUtils.validateResults(analysisResults, 2);
});
test('TypeIgnore5', () => {
const configOptions = new ConfigOptions('.');
let analysisResults = TestUtils.typeAnalyzeSampleFiles(['typeIgnore5.py'], configOptions);
TestUtils.validateResults(analysisResults, 0);
configOptions.diagnosticRuleSet.reportUnnecessaryTypeIgnoreComment = 'warning';
analysisResults = TestUtils.typeAnalyzeSampleFiles(['typeIgnore5.py'], configOptions);
TestUtils.validateResults(analysisResults, 0, 1);
});
test('DuplicateImports1', () => {
const configOptions = new ConfigOptions('.');

10
packages/pyright-internal/src/tests/samples/typeIgnore4.py Normal file
View File

@ -0,0 +1,10 @@
# This sample tests the reportUnnecessaryTypeIgnoreComment diagnostic check
# as applied to the entire file.
a: str = 3 # type: ignore
# This should emit an error if reportUnnecessaryTypeComment is enabled
b: str = "" # type: ignore
# This should emit an error if reportUnnecessaryTypeComment is enabled
c: int = 3 # type: ignore

7
packages/pyright-internal/src/tests/samples/typeIgnore5.py Normal file
View File

@ -0,0 +1,7 @@
# This sample tests the reportUnnecessaryTypeIgnoreComment diagnostic check
# as applied to individual lines.
# This should generate an error if reportUnnecessaryTypeIgnoreComment is enabled.
# type: ignore
b: str = ""

18
packages/pyright-internal/src/tests/tokenizer.test.ts
View File

@ -1455,40 +1455,40 @@ test('Identifiers1', () => {
test('TypeIgnoreAll1', () => {
const t = new Tokenizer();
const results = t.tokenize('\n#type:ignore\n"test"');
assert.equal(results.typeIgnoreAll, true);
assert(results.typeIgnoreAll);
});
test('TypeIgnoreAll2', () => {
const t = new Tokenizer();
const results = t.tokenize('\n# type: ignore ssss\n');
assert.equal(results.typeIgnoreAll, true);
assert(results.typeIgnoreAll);
});
test('TypeIgnoreAll3', () => {
const t = new Tokenizer();
const results = t.tokenize('\n# type: ignoressss\n');
assert.equal(results.typeIgnoreAll, false);
assert(!results.typeIgnoreAll);
});
test('TypeIgnoreAll3', () => {
const t = new Tokenizer();
const results = t.tokenize('\n"hello"\n# type: ignore\n');
assert.equal(results.typeIgnoreAll, false);
assert(!results.typeIgnoreAll);
});
test('TypeIgnoreLine1', () => {
const t = new Tokenizer();
const results = t.tokenize('\na = 3 # type: ignore\n"test" # type:ignore');
assert.equal(Object.keys(results.typeIgnoreLines).length, 2);
assert.equal(results.typeIgnoreLines[1], true);
assert.equal(results.typeIgnoreLines[2], true);
assert.equal(results.typeIgnoreLines.size, 2);
assert(results.typeIgnoreLines.has(1));
assert(results.typeIgnoreLines.has(2));
});
test('TypeIgnoreLine2', () => {
const t = new Tokenizer();
const results = t.tokenize('a = 3 # type: ignores\n"test" # type:ignore');
assert.equal(Object.keys(results.typeIgnoreLines).length, 1);
assert.equal(results.typeIgnoreLines[1], true);
assert.equal(results.typeIgnoreLines.size, 1);
assert(results.typeIgnoreLines.has(1));
assert.equal(results.tokens.getItemAtPosition(0), 0);
assert.equal(results.tokens.getItemAtPosition(1), 0);

17
packages/vscode-pyright/package.json
View File

@ -674,6 +674,17 @@
"error"
]
},
"reportUnsupportedDunderAll": {
"type": "string",
"description": "Diagnostics for unsupported operations performed on __all__.",
"default": "warning",
"enum": [
"none",
"information",
"warning",
"error"
]
},
"reportUnusedCallResult": {
"type": "string",
"description": "Diagnostics for call expressions whose results are not consumed and are not None.",
@ -696,10 +707,10 @@
"error"
]
},
"reportUnsupportedDunderAll": {
"reportUnnecessaryTypeIgnoreComment": {
"type": "string",
"description": "Diagnostics for unsupported operations performed on __all__.",
"default": "warning",
"description": "Diagnostics for '# type: ignore' comments that have no effect.",
"default": "none",
"enum": [
"none",
"information",

6
packages/vscode-pyright/schemas/pyrightconfig.schema.json
View File

@ -455,6 +455,12 @@
"title": "Controls reporting of call expressions that returns Coroutine whose results are not consumed",
"default": "error"
},
"reportUnnecessaryTypeIgnoreComment": {
"$id": "#/properties/reportUnnecessaryTypeIgnoreComment",
"$ref": "#/definitions/diagnostic",
"title": "Controls reporting of '# type: ignore' comments that have no effect'",
"default": "none"
},
"extraPaths": {
"$id": "#/properties/extraPaths",
"type": "array",