Skip to content

Commit

Permalink
simplify hint diagnostics into a single separate diagnostic category …
Browse files Browse the repository at this point in the history
…and report all baselined diagnostics with it
  • Loading branch information
DetachHead committed Nov 3, 2024
1 parent 4eeb4ef commit c324053
Show file tree
Hide file tree
Showing 22 changed files with 244 additions and 357 deletions.
22 changes: 19 additions & 3 deletions docs/configuration.md
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,23 @@ The following settings determine how different types should be evaluated.

- <a name="disableBytesTypePromotions"></a> **disableBytesTypePromotions** [boolean]: Disables legacy behavior where `bytearray` and `memoryview` are considered subtypes of `bytes`. [PEP 688](https://peps.python.org/pep-0688/#no-special-meaning-for-bytes) deprecates this behavior, but this switch is provided to restore the older behavior.

## Diagnostic Categories

diagnostics can be configured to be reported as any of the following categories:

- `"error"` - causes the CLI to fail with exit code 1
- `"warning"` - only causes the CLI to fail if [`failOnWarnings`](#failOnWarnings) is enabled or the [`--warnings`](./command-line.md#command-line) argument is used
- `"information"` - never causes the CLI to fail
- `"hint"` - only appears as a hint in the language server, not reported in the CLI at all. [baselined diagnostics](../benefits-over-pyright/baseline.md) are reported as hints

!!! note
the `"unreachable"`, `"unused"` and `"deprecated"` diagnostic categories are deprecated in favor of `"hint"`. rules where it makes sense
to be report them as "unnecessary" or "deprecated" [as mentioned in the LSP spec](https://microsoft.github.io/language-server-protocol/specifications/lsp/3.17/specification/#diagnosticSeverity) are still reported as such, the configuration to do so has just been simplified.

the `"hint"` diagnostic category is more flexible as it can be used on rules that don't refer to something that's unused, unreachable or deprecated. [baselined diagnostics](../benefits-over-pyright/baseline.md) are now all reported as a hint, instead of just the ones that supported one of the specific diagnostic tag categories.

for backwards compatibility, setting a diagnostic rule to any of these three deprecated categories will act as an alias for the `"hint"` category, however they may be removed entirely in a future release.

## Type Check Diagnostics Settings
The following settings control pyright’s diagnostic output (warnings or errors).

Expand All @@ -80,7 +97,7 @@ The following settings control pyright’s diagnostic output (warnings or errors

### Type Check Rule Overrides

The following settings allow more fine grained control over the **typeCheckingMode**. Unless otherwise specified, each diagnostic setting can specify a boolean value (`false` indicating that no error is generated and `true` indicating that an error is generated). Alternatively, a string value of `"none"`, `"warning"`, `"information"`, or `"error"` can be used to specify the diagnostic level.
The following settings allow more fine grained control over the **typeCheckingMode**. Unless otherwise specified, each diagnostic setting can specify a boolean value (`false` indicating that no error is generated and `true` indicating that an error is generated). Alternatively, a string value of `"none"`, `"hint"`, `"warning"`, `"information"`, or `"error"` can be used to specify the diagnostic level. [see above for more information](#diagnostic-categories)

- <a name="reportGeneralTypeIssues"></a> **reportGeneralTypeIssues** [boolean or string, optional]: Generate or suppress diagnostics for general type inconsistencies, unsupported operations, argument/parameter mismatches, etc. This covers all of the basic type-checking rules not covered by other rules. It does not include syntax errors.

Expand Down Expand Up @@ -387,11 +404,10 @@ executionEnvironments = [

Each diagnostic setting has a default that is dictated by the specified type checking mode. The default for each rule can be overridden in the configuration file or settings.

Some rules have an additional severity level such as `"unused"`, `"deprecated"` or `"unreachable"`. These are only used by the language server so that your editor can grey out or add a strikethrough to the symbol, which you can disable by setting it to `"off"`. it does not effect the outcome when running basedpyright via the CLI, so in that context these severity levels essentially mean the same thing as `"off"`.
Some rules default to `"hint"`. This diagnostic category is only used by the language server so that your editor can grey out or add a strikethrough to the symbol, which you can disable by setting it to `"off"`. it does not effect the outcome when running basedpyright via the CLI, so in that context these severity levels essentially mean the same thing as `"off"`. [see here](#diagnostic-categories) for more information about each diagnostic category.

The following table lists the default severity levels for each diagnostic rule within each type checking mode (`"off"`, `"basic"`, `"standard"`, `"strict"`, `"recommended"` and `"all"`).


### `"recommended"` and `"all"`

basedpyright introduces two new diagnostic rulesets in addition to the ones in pyright: `"recommended"` and `"all"`. `"recommended"` enables all diagnostic rules as either `"warning"` or `"error"`, but sets `failOnWarnings` to `true` so that all diagnostics will still cause a non-zero exit code when run in the CLI. this means `"recommended"` is essentially the same as `"all"`, but makes it easier to differentiate errors that are likely to cause a runtime crash like an undefined variable from less serious warnings such as a missing type annotation.
Expand Down
8 changes: 3 additions & 5 deletions packages/pyright-internal/src/analyzer/binder.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@

import { Commands } from '../commands/commands';
import { appendArray } from '../common/collectionUtils';
import { LspDiagnosticLevel } from '../common/configOptions';
import { DiagnosticLevel } from '../common/configOptions';
import { assert, assertNever, fail } from '../common/debug';
import { CreateTypeStubFileAction, Diagnostic, DiagnosticAddendum } from '../common/diagnostic';
import { DiagnosticRule } from '../common/diagnosticRules';
Expand Down Expand Up @@ -4283,16 +4283,14 @@ export class Binder extends ParseTreeWalker {
}

private _addDiagnostic(rule: DiagnosticRule, message: string, textRange: TextRange) {
const diagLevel = this._fileInfo.diagnosticRuleSet[rule] as LspDiagnosticLevel;
const diagLevel = this._fileInfo.diagnosticRuleSet[rule] as DiagnosticLevel;

let diagnostic: Diagnostic | undefined;
switch (diagLevel) {
case 'error':
case 'warning':
case 'information':
case 'unreachable':
case 'unused':
case 'deprecated':
case 'hint':
diagnostic = this._fileInfo.diagnosticSink.addDiagnosticWithTextRange(diagLevel, message, textRange);
break;

Expand Down
7 changes: 1 addition & 6 deletions packages/pyright-internal/src/analyzer/program.ts
Original file line number Diff line number Diff line change
Expand Up @@ -894,12 +894,7 @@ export class Program {
if (diagnostics !== undefined) {
// Filter out all categories that are translated to tagged hints?
if (options.disableTaggedHints) {
diagnostics = diagnostics.filter(
(diag) =>
diag.category !== DiagnosticCategory.UnreachableCode &&
diag.category !== DiagnosticCategory.UnusedCode &&
diag.category !== DiagnosticCategory.Deprecated
);
diagnostics = diagnostics.filter((diag) => diag.category !== DiagnosticCategory.Hint);
}

fileDiagnostics.push({
Expand Down
41 changes: 20 additions & 21 deletions packages/pyright-internal/src/analyzer/sourceFile.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,12 @@ import { isMainThread } from '../common/workersHost';

import { OperationCanceledException } from '../common/cancellationUtils';
import { appendArray } from '../common/collectionUtils';
import { ConfigOptions, ExecutionEnvironment, getBasicDiagnosticRuleSet } from '../common/configOptions';
import {
ConfigOptions,
ExecutionEnvironment,
getBasicDiagnosticRuleSet,
unreachableDiagnosticRules,
} from '../common/configOptions';
import { ConsoleInterface, StandardConsole } from '../common/console';
import { assert } from '../common/debug';
import { Diagnostic, DiagnosticCategory, TaskListToken, convertLevelToCategory } from '../common/diagnostic';
Expand Down Expand Up @@ -1018,11 +1023,7 @@ export class SourceFile {
if (this._diagnosticRuleSet.enableTypeIgnoreComments) {
if (this._writableData.typeIgnoreLines.size > 0) {
diagList = diagList.filter((d) => {
if (
d.category !== DiagnosticCategory.UnusedCode &&
d.category !== DiagnosticCategory.UnreachableCode &&
d.category !== DiagnosticCategory.Deprecated
) {
if (d.category !== DiagnosticCategory.Hint) {
for (let line = d.range.start.line; line <= d.range.end.line; line++) {
if (this._writableData.typeIgnoreLines.has(line)) {
typeIgnoreLinesClone.delete(line);
Expand All @@ -1039,11 +1040,7 @@ export class SourceFile {
// Filter the diagnostics based on "pyright: ignore" lines.
if (this._writableData.pyrightIgnoreLines.size > 0) {
diagList = diagList.filter((d) => {
if (
d.category !== DiagnosticCategory.UnusedCode &&
d.category !== DiagnosticCategory.UnreachableCode &&
d.category !== DiagnosticCategory.Deprecated
) {
if (d.category !== DiagnosticCategory.Hint) {
for (let line = d.range.start.line; line <= d.range.end.line; line++) {
const pyrightIgnoreComment = this._writableData.pyrightIgnoreLines.get(line);
if (pyrightIgnoreComment) {
Expand Down Expand Up @@ -1137,13 +1134,20 @@ export class SourceFile {
diag.category === DiagnosticCategory.Information
);

const unreachableDiagnostics = unreachableDiagnosticRules();
const isUnreachableCodeRange = (range: Range) => {
return prefilteredDiagList.find(
(diag) =>
diag.category === DiagnosticCategory.UnreachableCode &&
return prefilteredDiagList.find((diag) => {
if (diag.category !== DiagnosticCategory.Hint) {
return false;
}
const rule = diag.getRule();
return (
rule &&
unreachableDiagnostics.includes(rule as DiagnosticRule) &&
diag.range.start.line <= range.start.line &&
diag.range.end.line >= range.end.line
);
);
});
};

if (prefilteredErrorList.length === 0 && this._writableData.typeIgnoreAll !== undefined) {
Expand Down Expand Up @@ -1256,12 +1260,7 @@ export class SourceFile {
// the errors and warnings, leaving only the unreachable code
// and deprecated diagnostics.
if (!includeWarningsAndErrors) {
diagList = diagList.filter(
(diag) =>
diag.category === DiagnosticCategory.UnusedCode ||
diag.category === DiagnosticCategory.UnreachableCode ||
diag.category === DiagnosticCategory.Deprecated
);
diagList = diagList.filter((diag) => diag.category === DiagnosticCategory.Hint);
}

// If the file is in the ignore list, clear the diagnostic list.
Expand Down
8 changes: 4 additions & 4 deletions packages/pyright-internal/src/analyzer/typeEvaluator.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ import { CancellationToken } from 'vscode-languageserver';

import { invalidateTypeCacheIfCanceled, throwIfCancellationRequested } from '../common/cancellationUtils';
import { appendArray } from '../common/collectionUtils';
import { DiagnosticRuleSet, LspDiagnosticLevel } from '../common/configOptions';
import { DiagnosticRuleSet, DiagnosticLevel } from '../common/configOptions';
import { ConsoleInterface } from '../common/console';
import { assert, assertNever, fail } from '../common/debug';
import { DiagnosticAddendum } from '../common/diagnostic';
Expand Down Expand Up @@ -3395,7 +3395,7 @@ export function createTypeEvaluator(
}

function addDiagnosticWithSuppressionCheck(
diagLevel: LspDiagnosticLevel,
diagLevel: DiagnosticLevel,
message: string,
node: ParseNode,
range?: TextRange,
Expand Down Expand Up @@ -3450,7 +3450,7 @@ export function createTypeEvaluator(

function addDiagnostic(rule: DiagnosticRule, message: string, node: ParseNode, range?: TextRange) {
const fileInfo = AnalyzerNodeInfo.getFileInfo(node);
const diagLevel = fileInfo.diagnosticRuleSet[rule] as LspDiagnosticLevel;
const diagLevel = fileInfo.diagnosticRuleSet[rule] as DiagnosticLevel;

if (diagLevel === 'none') {
return undefined;
Expand Down Expand Up @@ -3504,7 +3504,7 @@ export function createTypeEvaluator(
message: string,
range: TextRange
) {
const diagLevel = fileInfo.diagnosticRuleSet[rule] as LspDiagnosticLevel;
const diagLevel = fileInfo.diagnosticRuleSet[rule] as DiagnosticLevel;

if (diagLevel === 'none') {
return undefined;
Expand Down
34 changes: 14 additions & 20 deletions packages/pyright-internal/src/baseline.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,7 @@
import { DiagnosticRule } from './common/diagnosticRules';
import { FileDiagnostics } from './common/diagnosticSink';
import { Uri } from './common/uri/uri';
import { compareDiagnostics, convertLevelToCategory, Diagnostic, isHintDiagnostic } from './common/diagnostic';
import { extraOptionDiagnosticRules } from './common/configOptions';
import { compareDiagnostics, Diagnostic, DiagnosticCategory } from './common/diagnostic';
import { fileExists } from './common/uri/uriUtils';
import { FileSystem } from './common/fileSystem';
import { pluralize } from './common/stringUtils';
Expand Down Expand Up @@ -126,7 +125,7 @@ export class BaselineHandler {
const newDiagnostics = filesWithDiagnostics.map((file) => ({
...file,
diagnostics: file.diagnostics.filter(
(diagnostic) => !diagnostic.baselineStatus && !isHintDiagnostic(diagnostic)
(diagnostic) => !diagnostic.baselined && diagnostic.category !== DiagnosticCategory.Hint
),
}));
if (newDiagnostics.map((fileWithDiagnostics) => fileWithDiagnostics.diagnostics.length).reduce(add, 0)) {
Expand Down Expand Up @@ -188,10 +187,11 @@ export class BaselineHandler {
assert(change.value[0] instanceof Diagnostic, "change object wasn't a Diagnostic");
result.push(...(change.value as Diagnostic[]));
} else {
// if not added and not removed
// if unchanged

// if the baselined error can be reported as a hint (eg. unreachable/deprecated), keep it and change its diagnostic
// level to that instead
// TODO: should we only baseline errors and not warnings/notes?
// TODO: should we only baseline errors/warnings and not notes?
for (const diagnostic of change.value) {
assert(
diagnostic instanceof Diagnostic,
Expand All @@ -200,20 +200,14 @@ export class BaselineHandler {
let newDiagnostic;
const diagnosticRule = diagnostic.getRule() as DiagnosticRule | undefined;
if (diagnosticRule) {
for (const { name, get } of extraOptionDiagnosticRules) {
if (get().includes(diagnosticRule)) {
newDiagnostic = diagnostic.copy({
category: convertLevelToCategory(name),
baselineStatus: 'baselined with hint',
});
newDiagnostic.setRule(diagnosticRule);
// none of these rules should have multiple extra diagnostic levels so we break after the first match
break;
}
}
newDiagnostic = diagnostic.copy({
category: DiagnosticCategory.Hint,
baselined: true,
});
newDiagnostic.setRule(diagnosticRule);
}
if (!newDiagnostic) {
newDiagnostic = diagnostic.copy({ baselineStatus: 'baselined' });
newDiagnostic = diagnostic.copy({ baselined: true });
}
result.push(newDiagnostic);
}
Expand All @@ -224,12 +218,12 @@ export class BaselineHandler {

/**
* filters out diagnostics that are baselined, but keeps any that have been turned into hints. so you will need
* to filter it further using {@link isHintDiagnostic} if you want those removed as well
* to filter it further by removing diagnostics with {@link DiagnosticCategory.Hint} if you want those removed as well
*/
filterOutBaselinedDiagnostics = (filesWithDiagnostics: readonly FileDiagnostics[]): readonly FileDiagnostics[] =>
filesWithDiagnostics.map((file) => ({
...file,
diagnostics: file.diagnostics.filter((diagnostic) => diagnostic.baselineStatus !== 'baselined'),
diagnostics: file.diagnostics.filter((diagnostic) => !diagnostic.baselined),
}));

private _getBaselinedErrorsForFile = (file: Uri): BaselinedDiagnostic[] => {
Expand All @@ -249,7 +243,7 @@ export class BaselineHandler {
for (const fileWithDiagnostics of filesWithDiagnostics) {
const filePath = this._rootDir.getRelativePath(fileWithDiagnostics.fileUri)!.toString();
const errorDiagnostics = fileWithDiagnostics.diagnostics.filter(
(diagnostic) => !isHintDiagnostic(diagnostic) || diagnostic.baselineStatus === 'baselined with hint'
(diagnostic) => diagnostic.category !== DiagnosticCategory.Hint || diagnostic.baselined
);
if (!(filePath in baselineData.files)) {
baselineData.files[filePath] = [];
Expand Down
8 changes: 2 additions & 6 deletions packages/pyright-internal/src/common/commandLineOptions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,9 +18,7 @@ export const enum DiagnosticSeverityOverrides {
Warning = 'warning',
Information = 'information',
None = 'none',
Unused = 'unused',
Unreachable = 'unreachable',
Deprecated = 'deprecated',
Hint = 'hint',
}

export function getDiagnosticSeverityOverrides() {
Expand All @@ -29,9 +27,7 @@ export function getDiagnosticSeverityOverrides() {
DiagnosticSeverityOverrides.Warning,
DiagnosticSeverityOverrides.Information,
DiagnosticSeverityOverrides.None,
DiagnosticSeverityOverrides.Unused,
DiagnosticSeverityOverrides.Unreachable,
DiagnosticSeverityOverrides.Deprecated,
DiagnosticSeverityOverrides.Hint,
];
}

Expand Down
Loading

0 comments on commit c324053

Please sign in to comment.