From 2d19ba9f77cba9e7e7a878ccea410dcd8b9bf963 Mon Sep 17 00:00:00 2001 From: Morgan Bartholomew Date: Mon, 15 Sep 2025 17:38:32 +1000 Subject: [PATCH 1/2] add diagnostic to prefer keyword arguments --- .../src/analyzer/typeEvaluator.ts | 24 +++++++++++++++++++ .../src/common/configOptions.ts | 9 +++++++ .../src/common/diagnosticRules.ts | 1 + .../tests/argumentsMatchParamNames.test.ts | 21 ++++++++++++++++ .../tests/samples/argumentsMatchParamNames.py | 23 ++++++++++++++++++ 5 files changed, 78 insertions(+) create mode 100644 packages/pyright-internal/src/tests/argumentsMatchParamNames.test.ts create mode 100644 packages/pyright-internal/src/tests/samples/argumentsMatchParamNames.py diff --git a/packages/pyright-internal/src/analyzer/typeEvaluator.ts b/packages/pyright-internal/src/analyzer/typeEvaluator.ts index 1bd02f1cc6..cb74bbcf3a 100644 --- a/packages/pyright-internal/src/analyzer/typeEvaluator.ts +++ b/packages/pyright-internal/src/analyzer/typeEvaluator.ts @@ -12287,6 +12287,30 @@ export function createTypeEvaluator( skipUnknownArgCheck = true; } + matchResults.argParams.forEach((argParam) => { + const a = argParam.argument; + if ( + a.argCategory === ArgCategory.Simple && + !a.name && + a.valueExpression && + a.valueExpression.nodeType === ParseNodeType.Name && + argParam.paramCategory === ParamCategory.Simple && + !argParam.mapsToVarArgList && + argParam.paramName && + !argParam.isParamNameSynthesized + ) { + const passedName = (a.valueExpression as NameNode).d.value; + const paramName = argParam.paramName; + if (passedName !== paramName) { + addDiagnostic( + DiagnosticRule.reportPositionalArgumentNameMismatch, + `Positional argument "${passedName}" does not match parameter name "${paramName}"; pass as a keyword argument to avoid confusion`, + a.valueExpression + ); + } + } + }); + // Run through all args and validate them against their matched parameter. // We'll do two phases. The first one establishes constraints for type // variables. The second perform type validation using the solved diff --git a/packages/pyright-internal/src/common/configOptions.ts b/packages/pyright-internal/src/common/configOptions.ts index 0b0a23dad9..1cf509dd15 100644 --- a/packages/pyright-internal/src/common/configOptions.ts +++ b/packages/pyright-internal/src/common/configOptions.ts @@ -204,6 +204,9 @@ export interface DiagnosticRuleSet { // Report issues related to call expressions? reportCallIssue: DiagnosticLevel; + // Report mismatch between positional argument names and parameter names when requested by decorator? + reportPositionalArgumentNameMismatch: DiagnosticLevel; + // Report inconsistencies with function overload signatures? reportInconsistentOverload: DiagnosticLevel; @@ -621,6 +624,7 @@ export function getOffDiagnosticRuleSet(): DiagnosticRuleSet { reportAssignmentType: 'none', reportAttributeAccessIssue: 'none', reportCallIssue: 'none', + reportPositionalArgumentNameMismatch: 'none', reportInconsistentOverload: 'none', reportIndexIssue: 'none', reportInvalidTypeArguments: 'none', @@ -740,6 +744,7 @@ export function getBasicDiagnosticRuleSet(): DiagnosticRuleSet { reportAssignmentType: 'error', reportAttributeAccessIssue: 'error', reportCallIssue: 'error', + reportPositionalArgumentNameMismatch: 'none', reportInconsistentOverload: 'error', reportIndexIssue: 'error', reportInvalidTypeArguments: 'error', @@ -859,6 +864,7 @@ export function getStandardDiagnosticRuleSet(): DiagnosticRuleSet { reportAssignmentType: 'error', reportAttributeAccessIssue: 'error', reportCallIssue: 'error', + reportPositionalArgumentNameMismatch: 'none', reportInconsistentOverload: 'error', reportIndexIssue: 'error', reportInvalidTypeArguments: 'error', @@ -1053,6 +1059,7 @@ export const getRecommendedDiagnosticRuleSet = (): DiagnosticRuleSet => ({ reportUnannotatedClassAttribute: 'warning', reportIncompatibleUnannotatedOverride: 'none', // TODO: change to error when we're confident there's no performance issues with this rule reportInvalidAbstractMethod: 'warning', + reportPositionalArgumentNameMismatch: 'warning', allowedUntypedLibraries: [], }); @@ -1168,6 +1175,7 @@ export const getAllDiagnosticRuleSet = (): DiagnosticRuleSet => ({ reportUnannotatedClassAttribute: 'error', reportIncompatibleUnannotatedOverride: 'error', reportInvalidAbstractMethod: 'error', + reportPositionalArgumentNameMismatch: 'error', allowedUntypedLibraries: [], }); @@ -1208,6 +1216,7 @@ export function getStrictDiagnosticRuleSet(): DiagnosticRuleSet { reportAssignmentType: 'error', reportAttributeAccessIssue: 'error', reportCallIssue: 'error', + reportPositionalArgumentNameMismatch: 'warning', reportInconsistentOverload: 'error', reportIndexIssue: 'error', reportInvalidTypeArguments: 'error', diff --git a/packages/pyright-internal/src/common/diagnosticRules.ts b/packages/pyright-internal/src/common/diagnosticRules.ts index c9ab083f33..be554ec28f 100644 --- a/packages/pyright-internal/src/common/diagnosticRules.ts +++ b/packages/pyright-internal/src/common/diagnosticRules.ts @@ -104,6 +104,7 @@ export enum DiagnosticRule { reportUnreachable = 'reportUnreachable', reportShadowedImports = 'reportShadowedImports', reportImplicitOverride = 'reportImplicitOverride', + reportPositionalArgumentNameMismatch = 'reportPositionalArgumentNameMismatch', // basedpyright options: failOnWarnings = 'failOnWarnings', diff --git a/packages/pyright-internal/src/tests/argumentsMatchParamNames.test.ts b/packages/pyright-internal/src/tests/argumentsMatchParamNames.test.ts new file mode 100644 index 0000000000..632875e611 --- /dev/null +++ b/packages/pyright-internal/src/tests/argumentsMatchParamNames.test.ts @@ -0,0 +1,21 @@ +import { ConfigOptions } from '../common/configOptions'; +import { Uri } from '../common/uri/uri'; +import { DiagnosticRule } from '../common/diagnosticRules'; +import * as TestUtils from './testUtils'; + +test('it reports an error', () => { + const configOptions = new ConfigOptions(Uri.empty()); + configOptions.diagnosticRuleSet.reportPositionalArgumentNameMismatch = 'warning'; + configOptions.diagnosticRuleSet.reportUnusedParameter = 'none'; + + const analysisResults = TestUtils.typeAnalyzeSampleFiles(['argumentsMatchParamNames.py'], configOptions); + + TestUtils.validateResultsButBased(analysisResults, { + warnings: [ + { + line: 11, + code: DiagnosticRule.reportPositionalArgumentNameMismatch, + }, + ], + }); +}); diff --git a/packages/pyright-internal/src/tests/samples/argumentsMatchParamNames.py b/packages/pyright-internal/src/tests/samples/argumentsMatchParamNames.py new file mode 100644 index 0000000000..a1cc5c0c3e --- /dev/null +++ b/packages/pyright-internal/src/tests/samples/argumentsMatchParamNames.py @@ -0,0 +1,23 @@ +def foo( + height: int, + width: int, +): ... + + +height = 1 +length = 2 + +foo( + height, # okay, name matches parameter name + length, # should display a warning, argument doesn't match parameter +) + +foo( + height=height, # okay, using keyword + width=length, # okay, using keyword +) + +foo( + height, # okay, matches + width=length, # okay, using keyword +) From fa43d2914aa60213ae4e68b469844b99c67ade5d Mon Sep 17 00:00:00 2001 From: KotlinIsland <65446343+kotlinisland@users.noreply.github.com> Date: Mon, 8 Dec 2025 18:42:19 +1000 Subject: [PATCH 2/2] wip --- .../tests/argumentsMatchParamNames.test.ts | 9 ++---- .../tests/samples/argumentsMatchParamNames.py | 30 ++++++++++++++++++- 2 files changed, 32 insertions(+), 7 deletions(-) diff --git a/packages/pyright-internal/src/tests/argumentsMatchParamNames.test.ts b/packages/pyright-internal/src/tests/argumentsMatchParamNames.test.ts index 632875e611..e09187c5c5 100644 --- a/packages/pyright-internal/src/tests/argumentsMatchParamNames.test.ts +++ b/packages/pyright-internal/src/tests/argumentsMatchParamNames.test.ts @@ -5,17 +5,14 @@ import * as TestUtils from './testUtils'; test('it reports an error', () => { const configOptions = new ConfigOptions(Uri.empty()); + configOptions.diagnosticRuleSet.reportUnnecessaryTypeIgnoreComment = 'error'; configOptions.diagnosticRuleSet.reportPositionalArgumentNameMismatch = 'warning'; configOptions.diagnosticRuleSet.reportUnusedParameter = 'none'; const analysisResults = TestUtils.typeAnalyzeSampleFiles(['argumentsMatchParamNames.py'], configOptions); TestUtils.validateResultsButBased(analysisResults, { - warnings: [ - { - line: 11, - code: DiagnosticRule.reportPositionalArgumentNameMismatch, - }, - ], + warnings: [], + errors: [], }); }); diff --git a/packages/pyright-internal/src/tests/samples/argumentsMatchParamNames.py b/packages/pyright-internal/src/tests/samples/argumentsMatchParamNames.py index a1cc5c0c3e..723717aa2a 100644 --- a/packages/pyright-internal/src/tests/samples/argumentsMatchParamNames.py +++ b/packages/pyright-internal/src/tests/samples/argumentsMatchParamNames.py @@ -1,3 +1,5 @@ +from typing import overload + def foo( height: int, width: int, @@ -9,7 +11,7 @@ def foo( foo( height, # okay, name matches parameter name - length, # should display a warning, argument doesn't match parameter + length, # pyright: ignore[reportPositionalArgumentNameMismatch] ) foo( @@ -21,3 +23,29 @@ def foo( height, # okay, matches width=length, # okay, using keyword ) + +def bar(a: int, /, b: int): ... + +bar( + 1, + 2, +) + +bar( + 1, + b=2, # pyright: ignore[reportPositionalArgumentNameMismatch] +) + +len([]) + +@overload +def f(a: int, b: int): ... +@overload +def f(a: str, b: str): ... + +def f(a, b): ... + +f( + 1, # pyright: ignore[reportPositionalArgumentNameMismatch] + 2, # pyright: ignore[reportPositionalArgumentNameMismatch] +)