diff --git a/packages/pyright-internal/src/analyzer/program.ts b/packages/pyright-internal/src/analyzer/program.ts index 29dbef7f9ee6..22439f0e9ed5 100644 --- a/packages/pyright-internal/src/analyzer/program.ts +++ b/packages/pyright-internal/src/analyzer/program.ts @@ -379,6 +379,14 @@ export class Program { this._console, this._logTracker ); + + // Set the initial diagnostic rule set from the execution environment + // so the file has config-level overrides (e.g. reportPrivateImportUsage: + // false) from the start. Without this, files added via positional args + // (which override configOptions.include) would use the basic defaults + // until parse() runs. + const execEnv = this._configOptions.findExecEnvironment(fileUri); + sourceFile.setInitialDiagnosticRuleSet(execEnv.diagnosticRuleSet); sourceFileInfo = new SourceFileInfo( sourceFile, sourceFile.isTypingStubFile() || sourceFile.isTypeshedStubFile() || sourceFile.isBuiltInStubFile(), diff --git a/packages/pyright-internal/src/analyzer/sourceFile.ts b/packages/pyright-internal/src/analyzer/sourceFile.ts index f8fafe90d127..776801ecabca 100644 --- a/packages/pyright-internal/src/analyzer/sourceFile.ts +++ b/packages/pyright-internal/src/analyzer/sourceFile.ts @@ -11,7 +11,12 @@ import { isMainThread } from 'worker_threads'; import { OperationCanceledException } from '../common/cancellationUtils'; import { appendArray } from '../common/collectionUtils'; -import { ConfigOptions, ExecutionEnvironment, getBasicDiagnosticRuleSet } from '../common/configOptions'; +import { + ConfigOptions, + DiagnosticRuleSet, + ExecutionEnvironment, + getBasicDiagnosticRuleSet, +} from '../common/configOptions'; import { ConsoleInterface, StandardConsole } from '../common/console'; import { assert } from '../common/debug'; import { Diagnostic, DiagnosticCategory, TaskListToken, convertLevelToCategory } from '../common/diagnostic'; @@ -314,6 +319,13 @@ export class SourceFile { this._ipythonMode = ipythonMode ?? IPythonMode.None; } + // Sets the initial diagnostic rule set from the execution environment's + // config-level overrides. This should be called immediately after + // construction so the file has the correct rules before parse/bind. + setInitialDiagnosticRuleSet(ruleSet: DiagnosticRuleSet) { + this._diagnosticRuleSet = { ...ruleSet }; + } + getIPythonMode(): IPythonMode { return this._ipythonMode; } diff --git a/packages/pyright-internal/src/tests/config.test.ts b/packages/pyright-internal/src/tests/config.test.ts index a3dffce6b067..ba3e5d203528 100644 --- a/packages/pyright-internal/src/tests/config.test.ts +++ b/packages/pyright-internal/src/tests/config.test.ts @@ -12,7 +12,12 @@ import assert from 'assert'; import { AnalyzerService } from '../analyzer/service'; import { deserialize, serialize } from '../backgroundThreadBase'; import { CommandLineOptions, DiagnosticSeverityOverrides } from '../common/commandLineOptions'; -import { ConfigOptions, ExecutionEnvironment, getStandardDiagnosticRuleSet } from '../common/configOptions'; +import { + ConfigOptions, + ExecutionEnvironment, + getBasicDiagnosticRuleSet, + getStandardDiagnosticRuleSet, +} from '../common/configOptions'; import { ConsoleInterface, NullConsole } from '../common/console'; import { TaskListPriority } from '../common/diagnostic'; import { combinePaths, normalizePath, normalizeSlashes } from '../common/pathUtils'; @@ -601,6 +606,39 @@ describe(`config test'}`, () => { assert.deepStrictEqual(config.defaultPythonVersion, pythonVersion3_13); }); + test('Diagnostic rule overrides are preserved when positional args override include', () => { + const cwd = normalizePath(combinePaths(process.cwd(), 'src/tests/samples/project_with_diag_overrides')); + const service = createAnalyzer(); + const commandLineOptions = new CommandLineOptions(cwd, /* fromLanguageServer */ false); + service.setOptions(commandLineOptions); + + // Get config without include override - should have reportPrivateImportUsage: 'none' + // because the config sets it to false. + const configWithoutOverride = service.test_getConfigOptions(commandLineOptions); + assert.equal(configWithoutOverride.diagnosticRuleSet.reportPrivateImportUsage, 'none'); + + // The basic default would be 'error', verify our config overrides it. + const basicDefaults = getBasicDiagnosticRuleSet(); + assert.equal(basicDefaults.reportPrivateImportUsage, 'error'); + + // Now simulate positional args overriding include (like `pyright --project config.json subdir`). + const commandLineOptionsWithOverride = new CommandLineOptions(cwd, /* fromLanguageServer */ false); + commandLineOptionsWithOverride.configSettings.includeFileSpecsOverride = [combinePaths(cwd, 'subdir')]; + service.setOptions(commandLineOptionsWithOverride); + + const configWithOverride = service.test_getConfigOptions(commandLineOptionsWithOverride); + + // The diagnostic rule overrides from the config file should still be applied + // even when positional args replace the include paths. + assert.equal(configWithOverride.diagnosticRuleSet.reportPrivateImportUsage, 'none'); + + // The execution environment for a file in the override path should also + // have the config's diagnostic rule overrides. + const fileUri = Uri.file(combinePaths(cwd, 'subdir', 'sample.py'), service.serviceProvider); + const execEnv = configWithOverride.findExecEnvironment(fileUri); + assert.equal(execEnv.diagnosticRuleSet.reportPrivateImportUsage, 'none'); + }); + function createAnalyzer(console?: ConsoleInterface) { const cons = console ?? new NullConsole(); const fs = createFromRealFileSystem(tempFile, cons); diff --git a/packages/pyright-internal/src/tests/privateImportUsage.test.ts b/packages/pyright-internal/src/tests/privateImportUsage.test.ts index bb63f59e6e83..e966e50fb1db 100644 --- a/packages/pyright-internal/src/tests/privateImportUsage.test.ts +++ b/packages/pyright-internal/src/tests/privateImportUsage.test.ts @@ -166,4 +166,177 @@ describe('reportPrivateImportUsage with tracked library files', () => { program2.dispose(); sp.dispose(); }); + + test('config file reportPrivateImportUsage override should apply when includeFileSpecsOverride is set', () => { + // This tests the scenario where pyright is run with positional directory arguments + // (e.g., pyright --project pyrightconfig.json dir1) and the config file has + // reportPrivateImportUsage: false. The override should still be respected. + + const files = [ + // pkg_a in library (defines the original function) + { + path: combinePaths(libraryRoot, 'pkg_a', '__init__.py'), + content: '', + }, + { + path: combinePaths(libraryRoot, 'pkg_a', 'py.typed'), + content: '', + }, + { + path: combinePaths(libraryRoot, 'pkg_a', 'utils.py'), + content: 'def helper_func(): pass', + }, + // pkg_b in library (re-imports without re-exporting) + { + path: combinePaths(libraryRoot, 'pkg_b', '__init__.py'), + content: '', + }, + { + path: combinePaths(libraryRoot, 'pkg_b', 'py.typed'), + content: '', + }, + { + path: combinePaths(libraryRoot, 'pkg_b', 'reexport.py'), + content: 'from pkg_a.utils import helper_func', // No __all__, not re-exported + }, + // Consumer package - local source file that imports from pkg_b + { + path: normalizeSlashes('/src/consumer/__init__.py'), + content: '', + }, + { + path: normalizeSlashes('/src/consumer/bad_import.py'), + content: 'from pkg_b.reexport import helper_func', // Would normally error + }, + ]; + + const sp = createServiceProviderFromFiles(files); + + // Test 1: With reportPrivateImportUsage = 'error' (default) -> should get error + { + const configOptions = new ConfigOptions(UriEx.file('/')); + configOptions.diagnosticRuleSet.reportPrivateImportUsage = 'error'; + + const importResolver = new ImportResolver( + sp, + configOptions, + new TestAccessHost(sp.fs().getModulePath(), [UriEx.file(libraryRoot)]) + ); + + const program = new Program(importResolver, configOptions, sp); + const consumerUri = UriEx.file('/src/consumer/bad_import.py'); + program.setTrackedFiles([consumerUri]); + + while (program.analyze()) { + // Continue until complete + } + + const sourceFile = program.getSourceFile(consumerUri); + assert(sourceFile, 'Source file should exist'); + const diagnostics = sourceFile.getDiagnostics(configOptions) || []; + const errors = diagnostics.filter((d) => d.category === DiagnosticCategory.Error); + + assert.strictEqual( + errors.length, + 1, + `Expected 1 error with reportPrivateImportUsage=error, got ${errors.length}` + ); + + program.dispose(); + } + + // Test 2: With reportPrivateImportUsage = 'none' (simulating config override) -> should NOT get error + { + const configOptions = new ConfigOptions(UriEx.file('/')); + configOptions.diagnosticRuleSet.reportPrivateImportUsage = 'none'; + + const importResolver = new ImportResolver( + sp, + configOptions, + new TestAccessHost(sp.fs().getModulePath(), [UriEx.file(libraryRoot)]) + ); + + const program = new Program(importResolver, configOptions, sp); + const consumerUri = UriEx.file('/src/consumer/bad_import.py'); + program.setTrackedFiles([consumerUri]); + + while (program.analyze()) { + // Continue until complete + } + + const sourceFile = program.getSourceFile(consumerUri); + assert(sourceFile, 'Source file should exist'); + const diagnostics = sourceFile.getDiagnostics(configOptions) || []; + const privateImportErrors = diagnostics.filter( + (d) => + d.category === DiagnosticCategory.Error && + (d.message.includes('not exported') || d.message.includes('helper_func')) + ); + + assert.strictEqual( + privateImportErrors.length, + 0, + `Expected 0 private import errors with reportPrivateImportUsage=none, got ${ + privateImportErrors.length + }: ${privateImportErrors.map((e) => e.message).join(', ')}` + ); + + program.dispose(); + } + + // Test 3: With reportPrivateImportUsage = 'none' AND positional arg include override + // This simulates: pyright --project config.json consumer/ + // where config.json has reportPrivateImportUsage: false + { + const configOptions = new ConfigOptions(UriEx.file('/')); + // Simulate the config file setting: first set standard mode defaults + configOptions.diagnosticRuleSet = ConfigOptions.getDiagnosticRuleSet('standard'); + // Then apply the config override (reportPrivateImportUsage: false -> 'none') + configOptions.diagnosticRuleSet.reportPrivateImportUsage = 'none'; + + // Simulate includeFileSpecsOverride by changing the include + // (In the real flow, this is done by _applyCommandLineOverrides) + configOptions.include = []; + + const importResolver = new ImportResolver( + sp, + configOptions, + new TestAccessHost(sp.fs().getModulePath(), [UriEx.file(libraryRoot)]) + ); + + const program = new Program(importResolver, configOptions, sp); + const consumerUri = UriEx.file('/src/consumer/bad_import.py'); + + // Track only the consumer file (simulating positional arg) + program.setTrackedFiles([consumerUri]); + + while (program.analyze()) { + // Continue until complete + } + + const sourceFile = program.getSourceFile(consumerUri); + assert(sourceFile, 'Source file should exist'); + const diagnostics = sourceFile.getDiagnostics(configOptions) || []; + const privateImportErrors = diagnostics.filter( + (d) => + d.category === DiagnosticCategory.Error && + (d.message.includes('not exported') || d.message.includes('helper_func')) + ); + + // The config says reportPrivateImportUsage: false, so there should be no errors + // even when include is overridden by positional args. + assert.strictEqual( + privateImportErrors.length, + 0, + `Expected 0 private import errors with config override and include override, got ${ + privateImportErrors.length + }: ${privateImportErrors.map((e) => e.message).join(', ')}. ` + + `This would indicate the bug where positional args cause config overrides to be ignored.` + ); + + program.dispose(); + } + + sp.dispose(); + }); }); diff --git a/packages/pyright-internal/src/tests/samples/project_with_diag_overrides/pyrightconfig.json b/packages/pyright-internal/src/tests/samples/project_with_diag_overrides/pyrightconfig.json new file mode 100644 index 000000000000..37d4f8736b0d --- /dev/null +++ b/packages/pyright-internal/src/tests/samples/project_with_diag_overrides/pyrightconfig.json @@ -0,0 +1,4 @@ +{ + "include": ["subdir"], + "reportPrivateImportUsage": false +} diff --git a/packages/pyright-internal/src/tests/samples/project_with_diag_overrides/subdir/sample.py b/packages/pyright-internal/src/tests/samples/project_with_diag_overrides/subdir/sample.py new file mode 100644 index 000000000000..f6488ba1e91c --- /dev/null +++ b/packages/pyright-internal/src/tests/samples/project_with_diag_overrides/subdir/sample.py @@ -0,0 +1 @@ +x: int = 1 diff --git a/packages/pyright-internal/src/tests/sourceFile.test.ts b/packages/pyright-internal/src/tests/sourceFile.test.ts index d54e3e988a01..444839285d96 100644 --- a/packages/pyright-internal/src/tests/sourceFile.test.ts +++ b/packages/pyright-internal/src/tests/sourceFile.test.ts @@ -10,7 +10,7 @@ import * as assert from 'assert'; import { ImportResolver } from '../analyzer/importResolver'; import { SourceFile } from '../analyzer/sourceFile'; -import { ConfigOptions } from '../common/configOptions'; +import { ConfigOptions, getBasicDiagnosticRuleSet, getOffDiagnosticRuleSet } from '../common/configOptions'; import { FullAccessHost } from '../common/fullAccessHost'; import { combinePaths } from '../common/pathUtils'; import { RealTempFile, createFromRealFileSystem } from '../common/realFileSystem'; @@ -34,6 +34,31 @@ test('Empty', () => { serviceProvider.dispose(); }); +test('SourceFile setInitialDiagnosticRuleSet overrides default', () => { + const filePath = combinePaths(process.cwd(), 'tests/samples/test_file1.py'); + const tempFile = new RealTempFile(); + const fs = createFromRealFileSystem(tempFile); + const serviceProvider = createServiceProvider(tempFile, fs); + + // Verify basic defaults have reportPrivateImportUsage as 'error'. + const basicRuleSet = getBasicDiagnosticRuleSet(); + assert.strictEqual(basicRuleSet.reportPrivateImportUsage, 'error'); + + // Create a rule set with reportPrivateImportUsage set to 'none'. + const offRuleSet = getOffDiagnosticRuleSet(); + assert.strictEqual(offRuleSet.reportPrivateImportUsage, 'none'); + + const sourceFile = new SourceFile(serviceProvider, Uri.file(filePath, serviceProvider), () => '', false, false, { + isEditMode: false, + }); + + // Call setInitialDiagnosticRuleSet to apply config-level overrides. + sourceFile.setInitialDiagnosticRuleSet(offRuleSet); + + assert.ok(sourceFile); + serviceProvider.dispose(); +}); + test('Empty Open file', () => { const code = ` // @filename: test.py