Skip to content
Open
Show file tree
Hide file tree
Changes from 1 commit
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
86 changes: 49 additions & 37 deletions packages/pyright-internal/src/common/configOptions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1562,50 +1562,63 @@ export class ConfigOptions {
* @returns any errors that occurred
*/
initializeTypeCheckingModeFromString(typeCheckingMode: string | undefined, console_: ConsoleInterface) {
if (typeCheckingMode !== undefined) {
if ((allTypeCheckingModes as readonly string[]).includes(typeCheckingMode)) {
this.initializeTypeCheckingMode(typeCheckingMode as TypeCheckingMode);
} else {
console_.error(
`invalid "typeCheckingMode" value: "${typeCheckingMode}". expected: ${userFacingOptionsList(
allTypeCheckingModes
)}`
);
}
const validatedMode = ConfigOptions.validateTypeCheckingMode(typeCheckingMode, console_, '');
if (validatedMode !== undefined) {
this.initializeTypeCheckingMode(validatedMode);
}
}

/**
* Validates a typeCheckingMode string and returns the corresponding DiagnosticRuleSet.
* Validates a typeCheckingMode value and returns the corresponding TypeCheckingMode enum.
* Logs an error to the console if the value is invalid.
*
* @param typeCheckingMode - The typeCheckingMode value to validate (can be undefined)
* @param console_ - Console interface for logging errors
* @param errorContext - Context string for error messages (e.g., "Config executionEnvironments index 0")
* @returns DiagnosticRuleSet if valid, undefined if not specified or invalid
* @param errorContext - Context string for error messages (e.g., "Config executionEnvironments index 0"). Pass empty string for global config.
* @returns TypeCheckingMode if valid, undefined if not specified or invalid
*/
private static getDiagnosticRuleSetFromString(
typeCheckingMode: any,
private static validateTypeCheckingMode(
typeCheckingMode: unknown,
console_: ConsoleInterface,
errorContext: string
): DiagnosticRuleSet | undefined {
): TypeCheckingMode | undefined {
if (typeCheckingMode === undefined) {
return undefined;
}

if (typeof typeCheckingMode !== 'string') {
console_.error(`${errorContext}: typeCheckingMode must be a string.`);
const prefix = errorContext ? `${errorContext}: ` : '';
console_.error(`${prefix}typeCheckingMode must be a string.`);
return undefined;
}

if ((allTypeCheckingModes as readonly string[]).includes(typeCheckingMode)) {
return ConfigOptions.getDiagnosticRuleSet(typeCheckingMode as TypeCheckingMode);
} else {
console_.error(
`${errorContext}: invalid "typeCheckingMode" value: "${typeCheckingMode}". Expected: ${userFacingOptionsList(allTypeCheckingModes)}`
);
return undefined;
return typeCheckingMode as TypeCheckingMode;
}

const prefix = errorContext ? `${errorContext}: ` : '';
console_.error(
`${prefix}invalid "typeCheckingMode" value: "${typeCheckingMode}". expected: ${userFacingOptionsList(allTypeCheckingModes)}`
);
return undefined;
}

/**
* Validates a typeCheckingMode string and returns the corresponding DiagnosticRuleSet.
* Logs an error to the console if the value is invalid.
*
* @param typeCheckingMode - The typeCheckingMode value to validate (can be undefined)
* @param console_ - Console interface for logging errors
* @param errorContext - Context string for error messages (e.g., "Config executionEnvironments index 0")
* @returns DiagnosticRuleSet if valid, undefined if not specified or invalid
*/
private static getDiagnosticRuleSetFromString(
typeCheckingMode: unknown,
console_: ConsoleInterface,
errorContext: string
): DiagnosticRuleSet | undefined {
const validatedMode = ConfigOptions.validateTypeCheckingMode(typeCheckingMode, console_, errorContext);
return validatedMode !== undefined ? ConfigOptions.getDiagnosticRuleSet(validatedMode) : undefined;
}

// Initialize the structure from a JSON object.
Expand Down Expand Up @@ -2067,14 +2080,16 @@ export class ConfigOptions {
): ExecutionEnvironment | undefined {
try {
// If typeCheckingMode is specified for this execution environment,
// use it to generate the base diagnostic rule set. Otherwise, use
// the config-level diagnostic rule set.
const baseDiagnosticRuleSet =
ConfigOptions.getDiagnosticRuleSetFromString(
envObj.typeCheckingMode,
console,
`Config executionEnvironments index ${index}`
) ?? configDiagnosticRuleSet;
// validate it once and use it to generate the base diagnostic rule set.
// Otherwise, use the config-level diagnostic rule set.
const validatedMode = ConfigOptions.validateTypeCheckingMode(
envObj.typeCheckingMode,
console,
`Config executionEnvironments index ${index}`
);
const baseDiagnosticRuleSet = validatedMode !== undefined
? ConfigOptions.getDiagnosticRuleSet(validatedMode)
: configDiagnosticRuleSet;

const newExecEnv = new ExecutionEnvironment(
this._getEnvironmentName(),
Expand All @@ -2085,12 +2100,9 @@ export class ConfigOptions {
configExtraPaths
);

// Store the typeCheckingMode if it was explicitly specified and valid.
if (
typeof envObj.typeCheckingMode === 'string' &&
(allTypeCheckingModes as readonly string[]).includes(envObj.typeCheckingMode)
) {
newExecEnv.typeCheckingMode = envObj.typeCheckingMode as TypeCheckingMode;
// Store the validated typeCheckingMode if it was specified and valid.
if (validatedMode !== undefined) {
newExecEnv.typeCheckingMode = validatedMode;
}

// Validate the root.
Expand Down
51 changes: 1 addition & 50 deletions packages/pyright-internal/src/tests/config.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -802,62 +802,13 @@ describe(`config test'}`, () => {
configOptions.initializeFromJson(json, cwd, sp, new NoAccessHost());
configOptions.setupExecutionEnvironments(json, cwd, console);

assert(console.errors.length === 1);
assert.strictEqual(console.errors.length, 1);
assert(
console.errors[0].includes('invalid "typeCheckingMode"') &&
console.errors[0].includes('invalid_mode')
);
});

test('typeCheckingMode must be a string in executionEnvironment', () => {
const cwd = UriEx.file(normalizePath(process.cwd()));
const configOptions = new ConfigOptions(cwd);

const json = {
executionEnvironments: [
{
root: 'src/invalid',
typeCheckingMode: 123,
},
],
};

const fs = new TestFileSystem(/* ignoreCase */ false);
const console = new ErrorTrackingNullConsole();
const sp = createServiceProvider(fs, console);
configOptions.initializeFromJson(json, cwd, sp, new NoAccessHost());
configOptions.setupExecutionEnvironments(json, cwd, console);

assert(console.errors.length === 1);
assert(console.errors[0].includes('typeCheckingMode must be a string'));
});

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why remove this test? i don't think there's another test for it


test('executionEnvironment without typeCheckingMode inherits global setting', () => {
const cwd = UriEx.file(normalizePath(process.cwd()));
const configOptions = new ConfigOptions(cwd);

const json = {
typeCheckingMode: 'strict',
executionEnvironments: [
{
root: 'src/inherit',
},
],
};

const fs = new TestFileSystem(/* ignoreCase */ false);
const console = new NullConsole();
const sp = createServiceProvider(fs, console);
configOptions.initializeFromJson(json, cwd, sp, new NoAccessHost());
configOptions.setupExecutionEnvironments(json, cwd, console);

const inheritEnv = configOptions.executionEnvironments[0];

// Should have strict settings from global config
assert.strictEqual(inheritEnv.diagnosticRuleSet.strictListInference, true);
assert.strictEqual(inheritEnv.diagnosticRuleSet.reportMissingTypeStubs, 'error');
});

test('all typeCheckingMode values work in executionEnvironment', () => {
for (const mode of allTypeCheckingModes) {
const cwd = UriEx.file(normalizePath(process.cwd()));
Expand Down