diff --git a/Extension/src/LanguageServer/configurations.ts b/Extension/src/LanguageServer/configurations.ts index 8a08b9987..ea135109d 100644 --- a/Extension/src/LanguageServer/configurations.ts +++ b/Extension/src/LanguageServer/configurations.ts @@ -24,6 +24,7 @@ import { PersistentFolderState } from './persistentState'; import { CppSettings, OtherSettings } from './settings'; import { SettingsPanel } from './settingsPanel'; import { ConfigurationType, getUI } from './ui'; +import { Deferral } from './utils'; import escapeStringRegExp = require('escape-string-regexp'); nls.config({ messageFormat: nls.MessageFormat.bundle, bundleFormat: nls.BundleFormat.standalone })(); @@ -152,7 +153,6 @@ export class CppProperties { private defaultCStandard: string | null = null; private defaultCppStandard: string | null = null; private defaultWindowsSdkVersion: string | null = null; - private isCppPropertiesJsonVisible: boolean = false; private vcpkgIncludes: string[] = []; private vcpkgPathReady: boolean = false; private nodeAddonIncludes: string[] = []; @@ -250,7 +250,6 @@ export class CppProperties { } public setupConfigurations(): void { - // defaultPaths is only used when there isn't a c_cpp_properties.json, but we don't send the configuration changed event // to the language server until the default include paths and frameworks have been sent. @@ -280,21 +279,21 @@ export class CppProperties { }); vscode.workspace.onDidChangeTextDocument((e) => { - if (e.document.uri.fsPath === settingsPath && this.isCppPropertiesJsonVisible) { - void this.handleSquiggles().catch(logAndReturn.undefined); + if (e.document.uri.fsPath === settingsPath) { + this.handleSquiggles(e.document); } }); - vscode.window.onDidChangeVisibleTextEditors((editors) => { - const wasVisible: boolean = this.isCppPropertiesJsonVisible; - editors.forEach(editor => { - if (editor.document.uri.fsPath === settingsPath) { - this.isCppPropertiesJsonVisible = true; - if (!wasVisible) { - void this.handleSquiggles().catch(logAndReturn.undefined); - } - } - }); + vscode.workspace.onDidOpenTextDocument(document => { + if (document.uri.fsPath === settingsPath) { + this.handleSquiggles(); + } + }); + + vscode.workspace.onDidCloseTextDocument(document => { + if (document.uri.fsPath === settingsPath) { + this.diagnosticCollection.clear(); + } }); vscode.workspace.onDidSaveTextDocument((doc: vscode.TextDocument) => { @@ -331,6 +330,7 @@ export class CppProperties { } }); } + public set CompilerDefaults(compilerDefaults: CompilerDefaults) { this.defaultCompilerPath = compilerDefaults.trustedCompilerFound ? compilerDefaults.compilerPath : null; this.knownCompilers = compilerDefaults.knownCompilers; @@ -353,7 +353,7 @@ export class CppProperties { private onSelectionChanged(): void { this.selectionChanged.fire(this.CurrentConfigurationIndex); - void this.handleSquiggles().catch(logAndReturn.undefined); + this.handleSquiggles(); } private onCompileCommandsChanged(path: string): void { @@ -493,9 +493,10 @@ export class CppProperties { }); } } - } catch (error) { /*ignore*/ } finally { + } catch (error) { + /*ignore*/ + } finally { this.vcpkgPathReady = true; - this.handleConfigurationChange(); } } @@ -795,7 +796,7 @@ export class CppProperties { return result; } - private resolveAndSplit(paths: string[] | undefined, defaultValue: string[] | undefined, env: Environment, assumeRelative: boolean = true, glob: boolean = false): string[] { + private async resolveAndSplit(paths: string[] | undefined, defaultValue: string[] | undefined, env: Environment, assumeRelative: boolean = true, glob: boolean = false): Promise { const resolvedVariables: string[] = []; if (paths === undefined) { return resolvedVariables; @@ -846,7 +847,7 @@ export class CppProperties { if (isGlobPattern) { // fastGlob silently strips non-found paths. Limit that behavior to dynamic paths only. const matches: string[] = fastGlob.isDynamicPattern(normalized) ? - fastGlob.sync(normalized, { onlyDirectories: true, cwd, suppressErrors: true, deep: 15 }) : [res]; + await fastGlob.async(normalized, { onlyDirectories: true, cwd, suppressErrors: true, deep: 15 }) : [res]; resolvedGlob.push(...matches.map(s => s + suffix)); if (resolvedGlob.length === 0) { resolvedGlob.push(normalized); @@ -881,14 +882,14 @@ export class CppProperties { return property; } - private updateConfigurationPathsArray(paths: string[] | undefined, defaultValue: string[] | undefined, env: Environment, assumeRelative: boolean = true): string[] | undefined { + private updateConfigurationPathsArray(paths: string[] | undefined, defaultValue: string[] | undefined, env: Environment, assumeRelative: boolean = true): Promise { if (paths) { return this.resolveAndSplit(paths, defaultValue, env, assumeRelative, true); } - if (!paths && defaultValue) { + if (defaultValue) { return this.resolveAndSplit(defaultValue, [], env, assumeRelative, true); } - return paths; + return Promise.resolve(undefined); } private updateConfigurationBoolean(property: boolean | string | undefined | null, defaultValue: boolean | undefined | null): boolean | undefined { @@ -933,7 +934,7 @@ export class CppProperties { return this.configProviderAutoSelected; } - private updateServerOnFolderSettingsChange(): void { + private async updateServerOnFolderSettingsChange(): Promise { this.configProviderAutoSelected = false; if (!this.configurationJson) { return; @@ -946,7 +947,7 @@ export class CppProperties { configuration.compilerPathInCppPropertiesJson = configuration.compilerPath; configuration.compileCommandsInCppPropertiesJson = configuration.compileCommands; configuration.configurationProviderInCppPropertiesJson = configuration.configurationProvider; - configuration.includePath = this.updateConfigurationPathsArray(configuration.includePath, settings.defaultIncludePath, env); + configuration.includePath = await this.updateConfigurationPathsArray(configuration.includePath, settings.defaultIncludePath, env); // in case includePath is reset below const origIncludePath: string[] | undefined = configuration.includePath; if (userSettings.addNodeAddonIncludePaths) { @@ -964,7 +965,7 @@ export class CppProperties { configuration.macFrameworkPath = this.updateConfigurationStringArray(configuration.macFrameworkPath, settings.defaultMacFrameworkPath, env); configuration.windowsSdkVersion = this.updateConfigurationString(configuration.windowsSdkVersion, settings.defaultWindowsSdkVersion, env); - configuration.forcedInclude = this.updateConfigurationPathsArray(configuration.forcedInclude, settings.defaultForcedInclude, env, false); + configuration.forcedInclude = await this.updateConfigurationPathsArray(configuration.forcedInclude, settings.defaultForcedInclude, env, false); configuration.compileCommands = this.updateConfigurationStringArray(configuration.compileCommands, settings.defaultCompileCommands, env); configuration.compilerArgs = this.updateConfigurationStringArray(configuration.compilerArgs, settings.defaultCompilerArgs, env); configuration.cStandard = this.updateConfigurationString(configuration.cStandard, settings.defaultCStandard, env); @@ -1042,7 +1043,7 @@ export class CppProperties { // Otherwise, if the browse path is not set, let the native process populate it // with include paths, including any parsed from compilerArgs. } else { - configuration.browse.path = this.updateConfigurationPathsArray(configuration.browse.path, settings.defaultBrowsePath, env); + configuration.browse.path = await this.updateConfigurationPathsArray(configuration.browse.path, settings.defaultBrowsePath, env); } configuration.browse.limitSymbolsToIncludedHeaders = this.updateConfigurationBoolean(configuration.browse.limitSymbolsToIncludedHeaders, settings.defaultLimitSymbolsToIncludedHeaders); @@ -1261,7 +1262,7 @@ export class CppProperties { this.settingsPanel.selectedConfigIndex = this.CurrentConfigurationIndex; this.settingsPanel.createOrShow(configNames, this.configurationJson.configurations[this.settingsPanel.selectedConfigIndex], - this.getErrorsForConfigUI(this.settingsPanel.selectedConfigIndex), + await this.getErrorsForConfigUI(this.settingsPanel.selectedConfigIndex), viewColumn); } } @@ -1292,7 +1293,7 @@ export class CppProperties { } this.settingsPanel.updateConfigUI(configNames, this.configurationJson.configurations[this.settingsPanel.selectedConfigIndex], - this.getErrorsForConfigUI(this.settingsPanel.selectedConfigIndex)); + await this.getErrorsForConfigUI(this.settingsPanel.selectedConfigIndex)); } else { // Parse failed, open json file void vscode.workspace.openTextDocument(this.propertiesFile).then(undefined, logAndReturn.undefined); @@ -1321,13 +1322,13 @@ export class CppProperties { return trimmedPaths; } - private saveConfigurationUI(): void { + private async saveConfigurationUI(): Promise { this.parsePropertiesFile(); // Clear out any modifications we may have made internally. if (this.settingsPanel && this.configurationJson) { const config: Configuration = this.settingsPanel.getLastValuesFromConfigUI(); this.configurationJson.configurations[this.settingsPanel.selectedConfigIndex] = config; this.configurationJson.configurations[this.settingsPanel.selectedConfigIndex].includePath = this.trimPathWhitespace(this.configurationJson.configurations[this.settingsPanel.selectedConfigIndex].includePath); - this.settingsPanel.updateErrors(this.getErrorsForConfigUI(this.settingsPanel.selectedConfigIndex)); + this.settingsPanel.updateErrors(await this.getErrorsForConfigUI(this.settingsPanel.selectedConfigIndex)); this.writeToJson(); } // Any time parsePropertiesFile is called, configurationJson gets @@ -1335,12 +1336,12 @@ export class CppProperties { this.handleConfigurationChange(); } - private onConfigSelectionChanged(): void { + private async onConfigSelectionChanged(): Promise { const configNames: string[] | undefined = this.ConfigurationNames; if (configNames && this.settingsPanel && this.configurationJson) { this.settingsPanel.updateConfigUI(configNames, this.configurationJson.configurations[this.settingsPanel.selectedConfigIndex], - this.getErrorsForConfigUI(this.settingsPanel.selectedConfigIndex)); + await this.getErrorsForConfigUI(this.settingsPanel.selectedConfigIndex)); } } @@ -1393,7 +1394,9 @@ export class CppProperties { } void this.applyDefaultIncludePathsAndFrameworks().catch(logAndReturn.undefined); - this.updateServerOnFolderSettingsChange(); + void this.timeOperation(() => this.updateServerOnFolderSettingsChange()).then(result => { + getOutputChannelLogger().appendLineAtLevel(5, localize('resolve.configuration.processed', "Processed c_cpp_properties.json in {0}s", result.duration)); + }).catch(logAndReturn.undefined); } private async ensurePropertiesFile(): Promise { @@ -1454,6 +1457,7 @@ export class CppProperties { return false; } let success: boolean = true; + const firstParse = this.configurationJson === undefined; try { const readResults: string = fs.readFileSync(this.propertiesFile.fsPath, 'utf8'); if (readResults === "") { @@ -1584,6 +1588,14 @@ export class CppProperties { e.recursiveIncludesOrderIsExplicit = e.recursiveIncludes?.order !== undefined; }); + if (firstParse) { + // Check documents that are already open since no event will fire for them. + const fsPath = this.propertiesFile.fsPath; + const document = vscode.workspace.textDocuments.find(doc => doc.uri.fsPath === fsPath); + if (document) { + this.handleSquiggles(document); + } + } } catch (errJS) { const err: Error = errJS as Error; const failedToParse: string = localize("failed.to.parse.properties", 'Failed to parse "{0}"', this.propertiesFile.fsPath); @@ -1712,7 +1724,21 @@ export class CppProperties { return compilerPathAndArgs; } - private getErrorsForConfigUI(configIndex: number): ConfigurationErrors { + /** + * Time an operation and return the result plus the length of the operation. + * + * @param operation The async operation to perform. + * @returns The result of the operation and the time it took to complete in seconds. + */ + private async timeOperation(operation: () => Promise): Promise<{ result: T | undefined; duration: number }> { + const start = process.hrtime(); + const result = await operation(); + const diff = process.hrtime(start); + const duration = diff[0] + diff[1] / 1e9; // diff[0] is in seconds, diff[1] is in nanoseconds + return { result, duration }; + } + + private async getErrorsForConfigUI(configIndex: number): Promise { const errors: ConfigurationErrors = {}; if (!this.configurationJson) { return errors; @@ -1731,15 +1757,20 @@ export class CppProperties { errors.compilerPath = compilerPathAndArgs.error; // Validate paths (directories) - errors.includePath = this.validatePath(config.includePath, { globPaths: true }); - errors.macFrameworkPath = this.validatePath(config.macFrameworkPath); - errors.browsePath = this.validatePath(config.browse ? config.browse.path : undefined); + try { + const { result, duration } = await this.timeOperation(() => this.validatePath(config.includePath, { globPaths: true })); + errors.includePath = result ?? duration >= 10 ? localize('resolve.includePath.took.too.long', "The include path validation took {0}s to evaluate", duration) : undefined; + } catch (e) { + errors.includePath = localize('resolve.includePath.failed', "Failed to resolve include path. Error: {0}", (e as Error).message); + } + errors.macFrameworkPath = await this.validatePath(config.macFrameworkPath); + errors.browsePath = await this.validatePath(config.browse ? config.browse.path : undefined); // Validate files - errors.forcedInclude = this.validatePath(config.forcedInclude, { isDirectory: false, assumeRelative: false }); - errors.compileCommands = this.validatePath(config.compileCommands, { isDirectory: false }); - errors.dotConfig = this.validatePath(config.dotConfig, { isDirectory: false }); - errors.databaseFilename = this.validatePath(config.browse ? config.browse.databaseFilename : undefined, { isDirectory: false }); + errors.forcedInclude = await this.validatePath(config.forcedInclude, { isDirectory: false, assumeRelative: false }); + errors.compileCommands = await this.validatePath(config.compileCommands, { isDirectory: false }); + errors.dotConfig = await this.validatePath(config.dotConfig, { isDirectory: false }); + errors.databaseFilename = await this.validatePath(config.browse ? config.browse.databaseFilename : undefined, { isDirectory: false }); // Validate intelliSenseMode if (isWindows) { @@ -1752,7 +1783,7 @@ export class CppProperties { return errors; } - private validatePath(input: string | string[] | undefined, { isDirectory = true, assumeRelative = true, globPaths = false } = {}): string | undefined { + private async validatePath(input: string | string[] | undefined, { isDirectory = true, assumeRelative = true, globPaths = false } = {}): Promise { if (!input) { return undefined; } @@ -1768,7 +1799,7 @@ export class CppProperties { } // Resolve and split any environment variables - paths = this.resolveAndSplit(paths, undefined, this.ExtendedEnvironment, assumeRelative, globPaths); + paths = await this.resolveAndSplit(paths, undefined, this.ExtendedEnvironment, assumeRelative, globPaths); for (const p of paths) { let pathExists: boolean = true; @@ -1834,7 +1865,24 @@ export class CppProperties { return errorMsg; } - private async handleSquiggles(): Promise { + private lastConfigurationVersion: number = 0; + private handleSquigglesDeferral: Deferral | undefined; + + private handleSquiggles(doc?: vscode.TextDocument): void { + // When we open the doc or the active config changes, we don't pass the doc in since we always want to process squiggles. + if (doc?.version !== this.lastConfigurationVersion) { + this.lastConfigurationVersion = doc?.version ?? 0; + + // Debounce the squiggles requests. + this.handleSquigglesDeferral?.cancel(); + this.handleSquigglesDeferral = new Deferral(() => void this.handleSquigglesImpl().catch(logAndReturn.undefined), 1000); + } + } + + /** + * Not to be called directly. Use the `handleSquiggles` method instead which will debounce the calls. + */ + private async handleSquigglesImpl(): Promise { if (!this.propertiesFile) { return; } @@ -1949,9 +1997,9 @@ export class CppProperties { curText = curText.substring(0, nextNameStart2); } if (this.prevSquiggleMetrics.get(currentConfiguration.name) === undefined) { - this.prevSquiggleMetrics.set(currentConfiguration.name, { PathNonExistent: 0, PathNotAFile: 0, PathNotADirectory: 0, CompilerPathMissingQuotes: 0, CompilerModeMismatch: 0, MultiplePathsNotAllowed: 0, MultiplePathsShouldBeSeparated: 0 }); + this.prevSquiggleMetrics.set(currentConfiguration.name, { PathNonExistent: 0, PathNotAFile: 0, PathNotADirectory: 0, SlowPathResolution: 0, CompilerPathMissingQuotes: 0, CompilerModeMismatch: 0, MultiplePathsNotAllowed: 0, MultiplePathsShouldBeSeparated: 0 }); } - const newSquiggleMetrics: { [key: string]: number } = { PathNonExistent: 0, PathNotAFile: 0, PathNotADirectory: 0, CompilerPathMissingQuotes: 0, CompilerModeMismatch: 0, MultiplePathsNotAllowed: 0, MultiplePathsShouldBeSeparated: 0 }; + const newSquiggleMetrics: { [key: string]: number } = { PathNonExistent: 0, PathNotAFile: 0, PathNotADirectory: 0, SlowPathResolution: 0, CompilerPathMissingQuotes: 0, CompilerModeMismatch: 0, MultiplePathsNotAllowed: 0, MultiplePathsShouldBeSeparated: 0 }; const isWindows: boolean = os.platform() === 'win32'; // TODO: Add other squiggles. @@ -2085,8 +2133,33 @@ export class CppProperties { // and extend that pattern to the next quote before and next quote after it. const pattern: RegExp = new RegExp(`"[^"]*?(?<="|;)${escapedPath}(?="|;).*?"`, "g"); const configMatches: string[] | null = curText.match(pattern); + let expandedPaths: string[]; - const expandedPaths: string[] = this.resolveAndSplit([curPath], undefined, this.ExtendedEnvironment, true, true); + try { + const { result, duration } = await this.timeOperation(() => this.resolveAndSplit([curPath], undefined, this.ExtendedEnvironment, true, true)); + expandedPaths = result ?? []; + if (duration > 10 && configMatches) { + newSquiggleMetrics.SlowPathResolution++; + const curOffset = curText.indexOf(configMatches[0]); + const endOffset = curOffset + curPath.length; + const diagnostic: vscode.Diagnostic = new vscode.Diagnostic( + new vscode.Range(document.positionAt(curTextStartOffset + curOffset), document.positionAt(curTextStartOffset + endOffset)), + localize('resolve.path.took.too.long', "Path took {0}s to evaluate", duration), + vscode.DiagnosticSeverity.Warning); + diagnostics.push(diagnostic); + } + } catch (e) { + expandedPaths = []; + if (configMatches) { + const curOffset = curText.indexOf(configMatches[0]); + const endOffset = curOffset + curPath.length; + const diagnostic: vscode.Diagnostic = new vscode.Diagnostic( + new vscode.Range(document.positionAt(curTextStartOffset + curOffset), document.positionAt(curTextStartOffset + endOffset)), + localize('resolve.path.failed', "Failed to resolve path {0}. Error: {1}", curPath, (e as Error).message), + vscode.DiagnosticSeverity.Warning); + diagnostics.push(diagnostic); + } + } const incorrectExpandedPaths: string[] = []; if (expandedPaths.length <= 0) { diff --git a/Extension/src/LanguageServer/dataBinding.ts b/Extension/src/LanguageServer/dataBinding.ts index 26a4691b7..5dcceaf5d 100644 --- a/Extension/src/LanguageServer/dataBinding.ts +++ b/Extension/src/LanguageServer/dataBinding.ts @@ -3,23 +3,7 @@ * See 'LICENSE' in the project root for license information. * ------------------------------------------------------------------------------------------ */ import * as vscode from 'vscode'; - -class Deferral { - private timer?: NodeJS.Timeout; - - constructor(callback: () => void, timeout: number) { - this.timer = setTimeout(() => { - this.timer = undefined; - callback(); - }, timeout); - } - public cancel() { - if (this.timer) { - clearTimeout(this.timer); - this.timer = undefined; - } - } -} +import { Deferral } from './utils'; export class DataBinding { private valueChanged = new vscode.EventEmitter(); diff --git a/Extension/src/LanguageServer/utils.ts b/Extension/src/LanguageServer/utils.ts index 3a46a486a..fff791eef 100644 --- a/Extension/src/LanguageServer/utils.ts +++ b/Extension/src/LanguageServer/utils.ts @@ -118,3 +118,23 @@ export async function checkDuration(fn: () => Promise): Promise<{ result: const result = await fn(); return { result, duration: performance.now() - start }; } + +/** + * A class that delays the execution of a callback until a specified timeout period has elapsed. + */ +export class Deferral { + private timer?: NodeJS.Timeout; + + constructor(callback: () => void, timeout: number) { + this.timer = setTimeout(() => { + this.timer = undefined; + callback(); + }, timeout); + } + public cancel() { + if (this.timer) { + clearTimeout(this.timer); + this.timer = undefined; + } + } +}