Skip to content
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
8 changes: 8 additions & 0 deletions packages/pyright-internal/src/analyzer/program.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
Expand Down
14 changes: 13 additions & 1 deletion 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 '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';
Expand Down Expand Up @@ -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;
}
Expand Down
40 changes: 39 additions & 1 deletion packages/pyright-internal/src/tests/config.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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);
Expand Down
173 changes: 173 additions & 0 deletions packages/pyright-internal/src/tests/privateImportUsage.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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();
});
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
{
"include": ["subdir"],
"reportPrivateImportUsage": false
}
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
x: int = 1
27 changes: 26 additions & 1 deletion packages/pyright-internal/src/tests/sourceFile.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand All @@ -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
Expand Down
Loading