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

fix typeCheckingMode not defaulting to "all" (was broken by a recent upstream change) #422

Merged
merged 1 commit into from
Jun 18, 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
2 changes: 1 addition & 1 deletion packages/pyright-internal/src/analyzer/service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -642,7 +642,7 @@ export class AnalyzerService {

const errors: string[] = [];

configOptions.initializeTypeCheckingMode(commandLineOptions.typeCheckingMode ?? 'standard');
configOptions.initializeTypeCheckingMode(commandLineOptions.typeCheckingMode);

let configs;
try {
Expand Down
2 changes: 1 addition & 1 deletion packages/pyright-internal/src/common/configOptions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1336,7 +1336,7 @@ export class ConfigOptions {
typeCheckingMode: string | undefined,
severityOverrides?: DiagnosticSeverityOverridesMap
) {
this.diagnosticRuleSet = ConfigOptions.getDiagnosticRuleSet(typeCheckingMode);
this.diagnosticRuleSet = this.constructor.getDiagnosticRuleSet(typeCheckingMode);

if (severityOverrides) {
this.applyDiagnosticOverrides(severityOverrides);
Expand Down
15 changes: 15 additions & 0 deletions packages/pyright-internal/src/tests/config.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -497,3 +497,18 @@ test('Extended config files', () => {
const configOptions = service.test_getConfigOptions(commandLineOptions);
assert.equal(configOptions.diagnosticRuleSet.strictListInference, true);
});

test('default typeCheckingMode should be "all"', () => {
const cwd = normalizePath(process.cwd());
const service = createAnalyzer();
const commandLineOptions = new CommandLineOptions(cwd, /* fromVsCodeExtension */ true);
commandLineOptions.configFilePath = 'src/tests/samples/project1';

const configOptions = service.test_getConfigOptions(commandLineOptions);
service.setOptions(commandLineOptions);

// as far as i can tell the typeCheckingMode value isn't saved anywhere, so we instead just check some options
// that are typically disabled by default unless typeCheckingMode is set to `"all"`
assert(configOptions.diagnosticRuleSet.deprecateTypingAliases);
assert.equal(configOptions.diagnosticRuleSet.reportAny, 'error');
});
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,12 @@ test('reportUnreachable TYPE_CHECKING', () => {
});

test('default typeCheckingMode=all', () => {
// there's a better test for this in `config.test.ts` which tests it in a more complete way.
// the logic for loading the config seems very convoluted and messy. the default typeCheckingMode
// seems to be determined multiple times. and there was an upstream change that broke our defaulting
// to "all" which went undetected by this test, because it was being changed to "standard" later on.
// i'm keeping both tests just in case, because i don't really get why it gets set multiple times.
// maybe this one effects something else
const configOptions = new BasedConfigOptions(Uri.empty());
const analysisResults = typeAnalyzeSampleFiles(['unreachable1.py'], configOptions);
validateResultsButBased(analysisResults, {
Expand Down
Loading