From e6ef6f8e2eecb4212ed8ee0c60d1048a8b17d468 Mon Sep 17 00:00:00 2001 From: Sandeep Somavarapu Date: Mon, 1 Sep 2025 11:51:23 +0200 Subject: [PATCH] fix handling not included properties (#264310) --- .../common/configurationModels.ts | 21 +- .../test/common/configurationModels.test.ts | 265 ++++++++++++++++++ 2 files changed, 277 insertions(+), 9 deletions(-) diff --git a/src/vs/platform/configuration/common/configurationModels.ts b/src/vs/platform/configuration/common/configurationModels.ts index edf1a427527..531820e4b85 100644 --- a/src/vs/platform/configuration/common/configurationModels.ts +++ b/src/vs/platform/configuration/common/configurationModels.ts @@ -14,7 +14,7 @@ import { IExtUri } from '../../../base/common/resources.js'; import * as types from '../../../base/common/types.js'; import { URI, UriComponents } from '../../../base/common/uri.js'; import { addToValueTree, ConfigurationTarget, getConfigurationValue, IConfigurationChange, IConfigurationChangeEvent, IConfigurationCompareResult, IConfigurationData, IConfigurationModel, IConfigurationOverrides, IConfigurationUpdateOverrides, IConfigurationValue, IInspectValue, IOverrides, removeFromValueTree, toValuesTree } from './configuration.js'; -import { ConfigurationScope, Extensions, IConfigurationPropertySchema, IConfigurationRegistry, overrideIdentifiersFromKey, OVERRIDE_PROPERTY_REGEX } from './configurationRegistry.js'; +import { ConfigurationScope, Extensions, IConfigurationPropertySchema, IConfigurationRegistry, overrideIdentifiersFromKey, OVERRIDE_PROPERTY_REGEX, IRegisteredConfigurationPropertySchema } from './configurationRegistry.js'; import { FileOperation, IFileService } from '../../files/common/files.js'; import { ILogService } from '../../log/common/log.js'; import { Registry } from '../../registry/common/platform.js'; @@ -405,8 +405,10 @@ export class ConfigurationModelParser { } protected doParseRaw(raw: any, options?: ConfigurationParseOptions): IConfigurationModel & { restricted?: string[]; hasExcludedProperties?: boolean } { - const configurationProperties = Registry.as(Extensions.Configuration).getConfigurationProperties(); - const filtered = this.filter(raw, configurationProperties, true, options); + const registry = Registry.as(Extensions.Configuration); + const configurationProperties = registry.getConfigurationProperties(); + const excludedConfigurationProperties = registry.getExcludedConfigurationProperties(); + const filtered = this.filter(raw, configurationProperties, excludedConfigurationProperties, true, options); raw = filtered.raw; const contents = toValuesTree(raw, message => this.logService.error(`Conflict in settings file ${this._name}: ${message}`)); const keys = Object.keys(raw); @@ -414,16 +416,16 @@ export class ConfigurationModelParser { return { contents, keys, overrides, restricted: filtered.restricted, hasExcludedProperties: filtered.hasExcludedProperties }; } - private filter(properties: any, configurationProperties: { [qualifiedKey: string]: IConfigurationPropertySchema | undefined }, filterOverriddenProperties: boolean, options?: ConfigurationParseOptions): { raw: {}; restricted: string[]; hasExcludedProperties: boolean } { + private filter(properties: any, configurationProperties: IStringDictionary, excludedConfigurationProperties: IStringDictionary, filterOverriddenProperties: boolean, options?: ConfigurationParseOptions): { raw: {}; restricted: string[]; hasExcludedProperties: boolean } { let hasExcludedProperties = false; - if (!options?.scopes && !options?.skipRestricted && !options?.exclude?.length) { + if (!options?.scopes && !options?.skipRestricted && !options?.skipUnregistered && !options?.exclude?.length) { return { raw: properties, restricted: [], hasExcludedProperties }; } const raw: any = {}; const restricted: string[] = []; for (const key in properties) { if (OVERRIDE_PROPERTY_REGEX.test(key) && filterOverriddenProperties) { - const result = this.filter(properties[key], configurationProperties, false, options); + const result = this.filter(properties[key], configurationProperties, excludedConfigurationProperties, false, options); raw[key] = result.raw; hasExcludedProperties = hasExcludedProperties || result.hasExcludedProperties; restricted.push(...result.restricted); @@ -432,7 +434,7 @@ export class ConfigurationModelParser { if (propertySchema?.restricted) { restricted.push(key); } - if (this.shouldInclude(key, propertySchema, options)) { + if (this.shouldInclude(key, propertySchema, excludedConfigurationProperties, options)) { raw[key] = properties[key]; } else { hasExcludedProperties = true; @@ -442,7 +444,7 @@ export class ConfigurationModelParser { return { raw, restricted, hasExcludedProperties }; } - private shouldInclude(key: string, propertySchema: IConfigurationPropertySchema | undefined, options: ConfigurationParseOptions): boolean { + private shouldInclude(key: string, propertySchema: IConfigurationPropertySchema | undefined, excludedConfigurationProperties: IStringDictionary, options: ConfigurationParseOptions): boolean { if (options.exclude?.includes(key)) { return false; } @@ -459,7 +461,8 @@ export class ConfigurationModelParser { return false; } - const scope = propertySchema ? typeof propertySchema.scope !== 'undefined' ? propertySchema.scope : ConfigurationScope.WINDOW : undefined; + const schema = propertySchema ?? excludedConfigurationProperties[key]; + const scope = schema ? typeof schema.scope !== 'undefined' ? schema.scope : ConfigurationScope.WINDOW : undefined; if (scope === undefined || options.scopes === undefined) { return true; } diff --git a/src/vs/platform/configuration/test/common/configurationModels.test.ts b/src/vs/platform/configuration/test/common/configurationModels.test.ts index 27e025bde09..a50178e7e81 100644 --- a/src/vs/platform/configuration/test/common/configurationModels.test.ts +++ b/src/vs/platform/configuration/test/common/configurationModels.test.ts @@ -101,6 +101,271 @@ suite('ConfigurationModelParser', () => { }); +suite('ConfigurationModelParser - Excluded Properties', () => { + + ensureNoDisposablesAreLeakedInTestSuite(); + + const configurationRegistry = Registry.as(Extensions.Configuration); + + let testConfigurationNodes: any[] = []; + + setup(() => reset()); + teardown(() => reset()); + + function reset() { + if (testConfigurationNodes.length > 0) { + configurationRegistry.deregisterConfigurations(testConfigurationNodes); + testConfigurationNodes = []; + } + } + + function registerTestConfiguration() { + const node = { + 'id': 'ExcludedPropertiesTest', + 'type': 'object', + 'properties': { + 'regularProperty': { + 'type': 'string' as const, + 'default': 'regular', + 'restricted': false + }, + 'restrictedProperty': { + 'type': 'string' as const, + 'default': 'restricted', + 'restricted': true + }, + 'excludedProperty': { + 'type': 'string' as const, + 'default': 'excluded', + 'restricted': true, + 'included': false + }, + 'excludedNonRestrictedProperty': { + 'type': 'string' as const, + 'default': 'excludedNonRestricted', + 'restricted': false, + 'included': false + } + } + }; + + configurationRegistry.registerConfiguration(node); + testConfigurationNodes.push(node); + return node; + } + + test('should handle excluded restricted properties correctly', () => { + registerTestConfiguration(); + + const testObject = new ConfigurationModelParser('test', new NullLogService()); + const testData = { + 'regularProperty': 'regularValue', + 'restrictedProperty': 'restrictedValue', + 'excludedProperty': 'excludedValue', + 'excludedNonRestrictedProperty': 'excludedNonRestrictedValue' + }; + + testObject.parse(JSON.stringify(testData), { skipRestricted: true }); + + assert.strictEqual(testObject.configurationModel.getValue('regularProperty'), 'regularValue'); + assert.strictEqual(testObject.configurationModel.getValue('restrictedProperty'), undefined); + assert.strictEqual(testObject.configurationModel.getValue('excludedProperty'), 'excludedValue'); + assert.strictEqual(testObject.configurationModel.getValue('excludedNonRestrictedProperty'), 'excludedNonRestrictedValue'); + assert.ok(testObject.restrictedConfigurations.includes('restrictedProperty')); + assert.ok(!testObject.restrictedConfigurations.includes('excludedProperty')); + }); + + test('should find excluded properties when checking for restricted settings', () => { + registerTestConfiguration(); + + const testObject = new ConfigurationModelParser('test', new NullLogService()); + const testData = { + 'excludedProperty': 'excludedValue' + }; + + testObject.parse(JSON.stringify(testData)); + + assert.strictEqual(testObject.configurationModel.getValue('excludedProperty'), 'excludedValue'); + assert.ok(!testObject.restrictedConfigurations.includes('excludedProperty')); + }); + + test('should handle override properties with excluded configurations', () => { + registerTestConfiguration(); + + const testObject = new ConfigurationModelParser('test', new NullLogService()); + const testData = { + '[typescript]': { + 'regularProperty': 'overrideRegular', + 'restrictedProperty': 'overrideRestricted', + 'excludedProperty': 'overrideExcluded' + } + }; + + testObject.parse(JSON.stringify(testData), { skipRestricted: true }); + + const overrideConfig = testObject.configurationModel.override('typescript'); + assert.strictEqual(overrideConfig.getValue('regularProperty'), 'overrideRegular'); + assert.strictEqual(overrideConfig.getValue('restrictedProperty'), undefined); + assert.strictEqual(overrideConfig.getValue('excludedProperty'), 'overrideExcluded'); + }); + + test('should handle scope filtering with excluded properties', () => { + const node = { + 'id': 'ScopeExcludedTest', + 'type': 'object', + 'properties': { + 'windowProperty': { + 'type': 'string' as const, + 'default': 'window', + 'scope': ConfigurationScope.WINDOW + }, + 'applicationProperty': { + 'type': 'string' as const, + 'default': 'application', + 'scope': ConfigurationScope.APPLICATION + }, + 'excludedApplicationProperty': { + 'type': 'string' as const, + 'default': 'excludedApplication', + 'scope': ConfigurationScope.APPLICATION, + 'included': false + } + } + }; + + configurationRegistry.registerConfiguration(node); + testConfigurationNodes.push(node); + + const testObject = new ConfigurationModelParser('test', new NullLogService()); + const testData = { + 'windowProperty': 'windowValue', + 'applicationProperty': 'applicationValue', + 'excludedApplicationProperty': 'excludedApplicationValue' + }; + + testObject.parse(JSON.stringify(testData), { scopes: [ConfigurationScope.WINDOW] }); + + assert.strictEqual(testObject.configurationModel.getValue('windowProperty'), 'windowValue'); + assert.strictEqual(testObject.configurationModel.getValue('applicationProperty'), undefined); + assert.strictEqual(testObject.configurationModel.getValue('excludedApplicationProperty'), undefined); + }); + + test('filter should handle include/exclude options with excluded properties', () => { + registerTestConfiguration(); + + const testObject = new ConfigurationModelParser('test', new NullLogService()); + const testData = { + 'regularProperty': 'regularValue', + 'excludedProperty': 'excludedValue' + }; + + testObject.parse(JSON.stringify(testData), { include: ['excludedProperty'] }); + + assert.strictEqual(testObject.configurationModel.getValue('regularProperty'), 'regularValue'); + assert.strictEqual(testObject.configurationModel.getValue('excludedProperty'), 'excludedValue'); + }); + + test('should handle exclude options with excluded properties', () => { + registerTestConfiguration(); + + const testObject = new ConfigurationModelParser('test', new NullLogService()); + const testData = { + 'regularProperty': 'regularValue', + 'excludedProperty': 'excludedValue' + }; + + testObject.parse(JSON.stringify(testData), { exclude: ['regularProperty'] }); + + assert.strictEqual(testObject.configurationModel.getValue('regularProperty'), undefined); + assert.strictEqual(testObject.configurationModel.getValue('excludedProperty'), 'excludedValue'); + }); + + test('should report hasExcludedProperties correctly when excluded properties are filtered', () => { + registerTestConfiguration(); + + const testObject = new ConfigurationModelParser('test', new NullLogService()); + const testData = { + 'regularProperty': 'regularValue', + 'restrictedProperty': 'restrictedValue', + 'excludedProperty': 'excludedValue' + }; + + testObject.parse(JSON.stringify(testData), { skipRestricted: true }); + + const model = testObject.configurationModel; + + assert.notStrictEqual(model.raw, undefined, 'Raw should be set when properties are excluded'); + }); + + test('skipUnregistered should exclude unregistered properties', () => { + registerTestConfiguration(); + + const testObject = new ConfigurationModelParser('test', new NullLogService()); + + testObject.parse(JSON.stringify({ + 'unregisteredProperty': 'value3' + }), { skipUnregistered: true }); + + assert.strictEqual(testObject.configurationModel.getValue('unregisteredProperty'), undefined); + }); + + test('shouldInclude method works correctly with excluded properties for skipUnregistered', () => { + registerTestConfiguration(); + + const testObject = new ConfigurationModelParser('test', new NullLogService()); + + testObject.parse(JSON.stringify({ + 'regularProperty': 'value1', + 'excludedProperty': 'value2', + 'unregisteredProperty': 'value3' + }), { skipUnregistered: true }); + + assert.strictEqual(testObject.configurationModel.getValue('regularProperty'), 'value1'); + assert.strictEqual(testObject.configurationModel.getValue('excludedProperty'), undefined); + assert.strictEqual(testObject.configurationModel.getValue('unregisteredProperty'), undefined); + }); + + test('excluded properties are found during property schema lookup', () => { + registerTestConfiguration(); + + const registry = Registry.as(Extensions.Configuration); + + const excludedProperties = registry.getExcludedConfigurationProperties(); + assert.ok(excludedProperties['excludedProperty'], 'Excluded property should be in excluded properties map'); + assert.ok(excludedProperties['excludedNonRestrictedProperty'], 'Excluded non-restricted property should be in excluded properties map'); + + const regularProperties = registry.getConfigurationProperties(); + assert.strictEqual(regularProperties['excludedProperty'], undefined, 'Excluded property should not be in regular properties map'); + assert.strictEqual(regularProperties['excludedNonRestrictedProperty'], undefined, 'Excluded non-restricted property should not be in regular properties map'); + + assert.ok(regularProperties['regularProperty'], 'Regular property should be in regular properties map'); + assert.ok(regularProperties['restrictedProperty'], 'Restricted property should be in regular properties map'); + }); + + test('should correctly use shouldInclude with excluded properties for scope and unregistered filtering', () => { + registerTestConfiguration(); + + const testObject = new ConfigurationModelParser('test', new NullLogService()); + const testData = { + 'regularProperty': 'regularValue', + 'restrictedProperty': 'restrictedValue', + 'excludedProperty': 'excludedValue', + 'excludedNonRestrictedProperty': 'excludedNonRestrictedValue', + 'unknownProperty': 'unknownValue' + }; + + testObject.parse(JSON.stringify(testData), { skipRestricted: true }); + + assert.strictEqual(testObject.configurationModel.getValue('regularProperty'), 'regularValue'); + assert.strictEqual(testObject.configurationModel.getValue('restrictedProperty'), undefined); + assert.ok(testObject.restrictedConfigurations.includes('restrictedProperty')); + assert.strictEqual(testObject.configurationModel.getValue('excludedProperty'), 'excludedValue'); + assert.ok(!testObject.restrictedConfigurations.includes('excludedProperty')); + assert.strictEqual(testObject.configurationModel.getValue('excludedNonRestrictedProperty'), 'excludedNonRestrictedValue'); + assert.strictEqual(testObject.configurationModel.getValue('unknownProperty'), 'unknownValue'); + }); +}); + suite('ConfigurationModel', () => { ensureNoDisposablesAreLeakedInTestSuite();