Skip to content
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

add reportUnusedParameter rule #589

Merged
merged 3 commits into from
Aug 17, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 11 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -180,6 +180,17 @@ pyright has the `reportMissingSuperCall` rule which, for this reason, complains

`reportUnsafeMultipleInheritance` bans multiple inheritance when there are multiple base classes with an `__init__` or `__new__` method, as there's no way to guarantee that all of them will get called with the correct arguments (or at all). this allows `reportMissingSuperCall` to be more lenient. ie. when `reportUnsafeMultipleInheritance` is enabled, missing `super()` calls will only be reported on classes that actually have a base class.

#### `reportUnusedParameter` - report errors on unused function parameters

pyright will report an unused diagnostic on unused function parameters:

```py
def print_value(value: str): # "value" is not accessed
print("something else")
```

but like with [unreachable code](#reportunreachable---report-errors-on-code-that-would-otherwise-be-completely-unchecked), this is greys out code instead of actually reporting it as an error. basedpyright introduces a new `reportUnusedParameter` diagnostic rule which supports all the severity options (`"error"`, `"warning"` and `"none"`) as well as `"unused"`, which is the default behavior in pyright.

### re-implementing pylance-exclusive features

basedpyright re-implements some of the features that microsoft made exclusive to pylance, which is microsoft's closed-source vscode extension built on top of the pyright language server with some additional exclusive functionality ([see the pylance FAQ for more information](https://github.com/microsoft/pylance-release/blob/main/FAQ.md#what-features-are-in-pylance-but-not-in-pyright-what-is-the-difference-exactly)).
Expand Down
3 changes: 3 additions & 0 deletions docs/configuration.md
Original file line number Diff line number Diff line change
Expand Up @@ -257,6 +257,8 @@ the following additional options are not available in regular pyright:

- <a name="reportUnsafeMultipleInheritance"></a> **reportUnsafeMultipleInheritance** [boolean or string, optional]: Generate or suppress diagnostics for classes that inherit from multiple base classes with an `__init__` or `__new__` method, which is unsafe because those additional constructors may either never get called or get called with invalid arguments.

- <a name="reportUnusedParameter"></a> **reportUnusedParameter** [boolean or string, optional]: Generate or suppress diagnostics for unused function parameters.

## Discouraged options

there are rules in pyright that are discouraged in basedpyright because we provide a better alternative. these options are still available for backwards compatibility, but you shouldn't use them.
Expand Down Expand Up @@ -480,6 +482,7 @@ note that some settings which are enabled by default in pyright are disabled by
| reportImplicitRelativeImport | "none" | "none" | "none" | "none" | "error" |
| reportInvalidCast | "none" | "none" | "none" | "none" | "error" |
| reportUnsafeMultipleInheritance | "none" | "none" | "none" | "none" | "error" |
| reportUnusedParameter | "unused" | "unused" | "unused" | "unused" | "error" |

## Overriding settings (in VS Code)

Expand Down
59 changes: 44 additions & 15 deletions packages/pyright-internal/src/analyzer/checker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@ import { CancellationToken } from 'vscode-languageserver';

import { Commands } from '../commands/commands';
import { appendArray } from '../common/collectionUtils';
import { LspDiagnosticLevel } from '../common/configOptions';
import { assert, assertNever } from '../common/debug';
import { ActionKind, Diagnostic, DiagnosticAddendum, RenameShadowedFileAction } from '../common/diagnostic';
import { DiagnosticRule } from '../common/diagnosticRules';
Expand Down Expand Up @@ -3592,14 +3591,12 @@ export class Checker extends ParseTreeWalker {
}

private _conditionallyReportUnusedDeclaration(decl: Declaration, isPrivate: boolean) {
let diagnosticLevel: LspDiagnosticLevel;
let nameNode: NameNode | undefined;
let message: string | undefined;
let rule: DiagnosticRule | undefined;

switch (decl.type) {
case DeclarationType.Alias:
diagnosticLevel = this._fileInfo.diagnosticRuleSet.reportUnusedImport;
rule = DiagnosticRule.reportUnusedImport;
if (decl.node.nodeType === ParseNodeType.ImportAs) {
if (decl.node.d.alias) {
Expand Down Expand Up @@ -3662,26 +3659,61 @@ export class Checker extends ParseTreeWalker {
return;
}

diagnosticLevel = this._fileInfo.diagnosticRuleSet.reportUnusedVariable;

if (decl.node.nodeType === ParseNodeType.Name) {
nameNode = decl.node;

// Don't emit a diagnostic if the name starts with an underscore.
// This indicates that the variable is unused.
if (nameNode.d.value.startsWith('_')) {
diagnosticLevel = 'none';
if (!nameNode.d.value.startsWith('_')) {
rule = DiagnosticRule.reportUnusedVariable;
}
} else if (decl.node.nodeType === ParseNodeType.Parameter) {
nameNode = decl.node.d.name;

// Don't emit a diagnostic for unused parameters or type parameters.
diagnosticLevel = 'none';
// check if it's an overridden method, in which case don't report unused parameters because the user has no choice
if (
nameNode &&
// parameters preficed with a single underscore mean intentionally unused, but parameters prefixed with a
// double underscore are the old way of defining positional-only parameters, in which case we still want to
// report an error if it's unused
!SymbolNameUtils.isProtectedName(nameNode.d.value) &&
decl.node.parent?.nodeType === ParseNodeType.Function
) {
const methodName = decl.node.parent.d.name;
// dunders typically need to be treated the same as overridden methods, which is unsafe and cringe, ideally
// they would always be overrides of an abstract method or something but whatever
if (!SymbolNameUtils.isDunderName(methodName.d.value)) {
const functionType = this._evaluator.getType(methodName);
if (functionType?.category === TypeCategory.Function && functionType.shared.methodClass) {
const classType = functionType.shared.methodClass;
if (
!classType.shared.baseClasses
.filter(isClass)
.some((mroBaseClass) =>
lookUpClassMember(
mroBaseClass,
methodName.d.value,
MemberAccessFlags.Default
)
)
) {
rule = DiagnosticRule.reportUnusedParameter;
}
} else {
rule = DiagnosticRule.reportUnusedParameter;
}
}
}
} else {
rule = DiagnosticRule.reportUnusedVariable;
}

if (nameNode) {
rule = DiagnosticRule.reportUnusedVariable;
message = LocMessage.unaccessedVariable().format({ name: nameNode.d.value });
message = (
rule === DiagnosticRule.reportUnusedParameter
? LocMessage.unaccessedSymbol()
: LocMessage.unaccessedVariable()
).format({ name: nameNode.d.value });
}
break;

Expand All @@ -3696,7 +3728,6 @@ export class Checker extends ParseTreeWalker {
return;
}

diagnosticLevel = this._fileInfo.diagnosticRuleSet.reportUnusedClass;
nameNode = decl.node.d.name;
rule = DiagnosticRule.reportUnusedClass;
message = LocMessage.unaccessedClass().format({ name: nameNode.d.value });
Expand All @@ -3713,15 +3744,13 @@ export class Checker extends ParseTreeWalker {
return;
}

diagnosticLevel = this._fileInfo.diagnosticRuleSet.reportUnusedFunction;
nameNode = decl.node.d.name;
rule = DiagnosticRule.reportUnusedFunction;
message = LocMessage.unaccessedFunction().format({ name: nameNode.d.value });
break;

case DeclarationType.TypeParam:
// Never report a diagnostic for an unused TypeParam.
diagnosticLevel = 'none';
nameNode = decl.node.d.name;
break;

Expand All @@ -3734,7 +3763,7 @@ export class Checker extends ParseTreeWalker {
}

const action = rule === DiagnosticRule.reportUnusedImport ? { action: Commands.unusedImport } : undefined;
if (nameNode && rule !== undefined && message && diagnosticLevel !== 'none') {
if (nameNode && rule !== undefined && message) {
const diagnostic = this._evaluator.addDiagnostic(rule, message, nameNode);
if (action) {
diagnostic?.addAction(action);
Expand Down
8 changes: 8 additions & 0 deletions packages/pyright-internal/src/common/configOptions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -404,6 +404,7 @@ export interface DiagnosticRuleSet {
reportImplicitRelativeImport: DiagnosticLevel;
reportInvalidCast: DiagnosticLevel;
reportUnsafeMultipleInheritance: DiagnosticLevel;
reportUnusedParameter: UnusedDiagnosticLevel;
}

export function cloneDiagnosticRuleSet(diagSettings: DiagnosticRuleSet): DiagnosticRuleSet {
Expand Down Expand Up @@ -528,6 +529,7 @@ export function getDiagLevelDiagnosticRules() {
DiagnosticRule.reportImplicitRelativeImport,
DiagnosticRule.reportPrivateLocalImportUsage,
DiagnosticRule.reportUnsafeMultipleInheritance,
DiagnosticRule.reportUnusedParameter,
];
}

Expand All @@ -549,6 +551,7 @@ const unusedDiagnosticRules: DiagnosticGetter = {
DiagnosticRule.reportUnusedImport,
DiagnosticRule.reportUnusedFunction,
DiagnosticRule.reportUnusedVariable,
DiagnosticRule.reportUnusedParameter,
],
};
const deprecatedDiagnosticRules: DiagnosticGetter = {
Expand Down Expand Up @@ -673,6 +676,7 @@ export function getOffDiagnosticRuleSet(): DiagnosticRuleSet {
reportImplicitRelativeImport: 'none',
reportInvalidCast: 'none',
reportUnsafeMultipleInheritance: 'none',
reportUnusedParameter: 'unused',
};

return diagSettings;
Expand Down Expand Up @@ -783,6 +787,7 @@ export function getBasicDiagnosticRuleSet(): DiagnosticRuleSet {
reportImplicitRelativeImport: 'none',
reportInvalidCast: 'none',
reportUnsafeMultipleInheritance: 'none',
reportUnusedParameter: 'unused',
};

return diagSettings;
Expand Down Expand Up @@ -893,6 +898,7 @@ export function getStandardDiagnosticRuleSet(): DiagnosticRuleSet {
reportImplicitRelativeImport: 'none',
reportInvalidCast: 'none',
reportUnsafeMultipleInheritance: 'none',
reportUnusedParameter: 'unused',
};

return diagSettings;
Expand Down Expand Up @@ -1002,6 +1008,7 @@ export const getAllDiagnosticRuleSet = (): DiagnosticRuleSet => ({
reportImplicitRelativeImport: 'error',
reportInvalidCast: 'error',
reportUnsafeMultipleInheritance: 'error',
reportUnusedParameter: 'error',
});

export function getStrictDiagnosticRuleSet(): DiagnosticRuleSet {
Expand Down Expand Up @@ -1109,6 +1116,7 @@ export function getStrictDiagnosticRuleSet(): DiagnosticRuleSet {
reportImplicitRelativeImport: 'none',
reportInvalidCast: 'none',
reportUnsafeMultipleInheritance: 'none',
reportUnusedParameter: 'unused',
};

return diagSettings;
Expand Down
1 change: 1 addition & 0 deletions packages/pyright-internal/src/common/diagnosticRules.ts
Original file line number Diff line number Diff line change
Expand Up @@ -112,4 +112,5 @@ export enum DiagnosticRule {
reportImplicitRelativeImport = 'reportImplicitRelativeImport',
reportInvalidCast = 'reportInvalidCast',
reportUnsafeMultipleInheritance = 'reportUnsafeMultipleInheritance',
reportUnusedParameter = 'reportUnusedParameter',
}
2 changes: 2 additions & 0 deletions packages/pyright-internal/src/localization/localize.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1104,6 +1104,8 @@ export namespace Localizer {
new ParameterizedString<{ name: string }>(getRawString('Diagnostic.unaccessedFunction'));
export const unaccessedImport = () =>
new ParameterizedString<{ name: string }>(getRawString('Diagnostic.unaccessedImport'));
export const unaccessedSymbol = () =>
new ParameterizedString<{ name: string }>(getRawString('Diagnostic.unaccessedSymbol'));
export const unaccessedVariable = () =>
new ParameterizedString<{ name: string }>(getRawString('Diagnostic.unaccessedVariable'));
export const unannotatedFunctionSkipped = () =>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -567,6 +567,7 @@
"unaccessedClass": "Třída „{name}“ není přístupná",
"unaccessedFunction": "Funkce {name} není přístupná",
"unaccessedImport": "Import {name} není přístupný",
"unaccessedSymbol": "{name} není přístupné",
"unaccessedVariable": "Proměnná {name} není přístupná",
"unannotatedFunctionSkipped": "Analýza funkce „{name}“ se přeskočila, protože není označená",
"unaryOperationNotAllowed": "Unární operátor není v poznámce typu povolený.",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -567,6 +567,7 @@
"unaccessedClass": "Auf die Klasse \"{name}\" kann nicht zugegriffen werden.",
"unaccessedFunction": "Auf die Funktion \"{name}\" kann nicht zugegriffen werden.",
"unaccessedImport": "Auf den Import \"{name}\" kann nicht zugegriffen werden.",
"unaccessedSymbol": "Auf \"{name}\" kann nicht zugegriffen werden.",
"unaccessedVariable": "Auf die Variable \"{name}\" kann nicht zugegriffen werden.",
"unannotatedFunctionSkipped": "Die Analyse der Funktion \"{name}\" wird übersprungen, da sie nicht kommentiert wurde.",
"unaryOperationNotAllowed": "Unärer Operator in Typanmerkung nicht zulässig",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -565,6 +565,7 @@
"unaccessedClass": "Class \"{name}\" is not accessed",
"unaccessedFunction": "Function \"{name}\" is not accessed",
"unaccessedImport": "Import \"{name}\" is not accessed",
"unaccessedSymbol": "\"{name}\" is not accessed",
"unaccessedVariable": "Variable \"{name}\" is not accessed",
"unannotatedFunctionSkipped": "Analysis of function \"{name}\" is skipped because it is unannotated",
"unaryOperationNotAllowed": "Unary operator not allowed in type annotation",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -567,6 +567,7 @@
"unaccessedClass": "No se accede a la clase \"{name}\"",
"unaccessedFunction": "No se accede a la función \"{name}",
"unaccessedImport": "No se accede a la importación \"{name}",
"unaccessedSymbol": "No se accede a \"{name}\"",
"unaccessedVariable": "No se accede a la variable \"{name} \".",
"unannotatedFunctionSkipped": "Se omite el análisis de la función \"{name}\" porque no está anotada",
"unaryOperationNotAllowed": "Operador unario no permitido en la anotación de tipo",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -567,6 +567,7 @@
"unaccessedClass": "La classe \"{name}\" n'est pas accessible",
"unaccessedFunction": "La fonction « {name} » n’est pas accessible",
"unaccessedImport": "L’importation « {name} » n’est pas accessible",
"unaccessedSymbol": "« {name} » n’est pas accessible",
"unaccessedVariable": "La variable « {name} » n’est pas accessible",
"unannotatedFunctionSkipped": "L'analyse de la fonction \"{name}\" est ignorée car elle n'est pas annotée",
"unaryOperationNotAllowed": "Opérateur unaire non autorisé dans l’annotation de type",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -567,6 +567,7 @@
"unaccessedClass": "La classe \"{name}\" non è accessibile",
"unaccessedFunction": "La classe \"{name}\" non è accessibile",
"unaccessedImport": "Non è possibile accedere all'importazione \"{name}\"",
"unaccessedSymbol": "Non è possibile accedere a \"{name}\"",
"unaccessedVariable": "La variabile \"{name}\" non è accessibile",
"unannotatedFunctionSkipped": "L'analisi della funzione \"{name}\" è stata ignorata perché non è annotata",
"unaryOperationNotAllowed": "Operatore unario non consentito nell'annotazione di tipo",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -567,6 +567,7 @@
"unaccessedClass": "クラス \"{name}\" にアクセスできません",
"unaccessedFunction": "関数 \"{name}\" にアクセスできません",
"unaccessedImport": "インポート \"{name}\" にアクセスできません",
"unaccessedSymbol": "\"{name}\" にアクセスできません",
"unaccessedVariable": "変数 \"{name}\" にアクセスできません",
"unannotatedFunctionSkipped": "関数 \"{name}\" の分析は、表示されないためスキップされます",
"unaryOperationNotAllowed": "型の注釈で単項演算子は使用できません",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -567,6 +567,7 @@
"unaccessedClass": "‘{name}’ 클래스에 액세스할 수 없습니다.",
"unaccessedFunction": "함수 \"{name}\"에 액세스할 수 없습니다.",
"unaccessedImport": "가져오기 \"{name}\"에 액세스할 수 없습니다.",
"unaccessedSymbol": "\"{name}\"에 액세스할 수 없습니다.",
"unaccessedVariable": "변수 \"{name}\"에 액세스할 수 없습니다.",
"unannotatedFunctionSkipped": "주석이 없으므로 ‘{name}’ 함수 분석을 건너뜁니다.",
"unaryOperationNotAllowed": "형식 주석에는 단항 연산자를 사용할 수 없습니다.",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -567,6 +567,7 @@
"unaccessedClass": "Nie uzyskano dostępu do klasy „{name}”",
"unaccessedFunction": "Brak dostępu do funkcji „{name}”.",
"unaccessedImport": "Import „{name}” nie jest dostępny",
"unaccessedSymbol": "Brak dostępu do „{name}”.",
"unaccessedVariable": "Brak dostępu do zmiennej „{name}”.",
"unannotatedFunctionSkipped": "Analiza funkcji „{name}” została pominięta, ponieważ nie ma adnotacji",
"unaryOperationNotAllowed": "Operator jednoargumentowy nie jest dozwolony w adnotacji typu",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -567,6 +567,7 @@
"unaccessedClass": "A classe \"{name}\" não foi acessada",
"unaccessedFunction": "A função \"{name}\" não foi acessada",
"unaccessedImport": "A importação \"{name}\" não foi acessada",
"unaccessedSymbol": "\"{name}\" não foi acessado",
"unaccessedVariable": "A variável \"{name}\" não foi acessada",
"unannotatedFunctionSkipped": "A análise da função \"{name}\" foi ignorada porque não foi anotada",
"unaryOperationNotAllowed": "Operador unário não permitido na anotação de tipo",
Expand Down
Loading
Loading