Skip to content

Commit 56576c8

Browse files
authored
refactor the compilerPath verification to get consistent results in JSON and UI verification (#13553)
1 parent 42a9eba commit 56576c8

File tree

12 files changed

+416
-174
lines changed

12 files changed

+416
-174
lines changed

Extension/.gitignore

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ server
1010
debugAdapters
1111
LLVM
1212
bin/cpptools*
13+
bin/libc.so
1314
bin/*.dll
1415
bin/.vs
1516
bin/LICENSE.txt

Extension/c_cpp_properties.schema.json

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -19,8 +19,8 @@
1919
"markdownDescription": "Full path of the compiler being used, e.g. `/usr/bin/gcc`, to enable more accurate IntelliSense.",
2020
"descriptionHint": "Markdown text between `` should not be translated or localized (they represent literal text) and the capitalization, spacing, and punctuation (including the ``) should not be altered.",
2121
"type": [
22-
"string",
23-
"null"
22+
"null",
23+
"string"
2424
]
2525
},
2626
"compilerArgs": {
@@ -312,4 +312,4 @@
312312
"version"
313313
],
314314
"additionalProperties": false
315-
}
315+
}

Extension/src/LanguageServer/configurations.ts

Lines changed: 101 additions & 124 deletions
Large diffs are not rendered by default.

Extension/src/LanguageServer/cppBuildTaskProvider.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -98,7 +98,7 @@ export class CppBuildTaskProvider implements TaskProvider {
9898

9999
// Get user compiler path.
100100
const userCompilerPathAndArgs: util.CompilerPathAndArgs | undefined = await activeClient.getCurrentCompilerPathAndArgs();
101-
let userCompilerPath: string | undefined | null;
101+
let userCompilerPath: string | undefined;
102102
if (userCompilerPathAndArgs) {
103103
userCompilerPath = userCompilerPathAndArgs.compilerPath;
104104
if (userCompilerPath && userCompilerPathAndArgs.compilerName) {

Extension/src/common.ts

Lines changed: 38 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -1092,74 +1092,66 @@ export function isCl(compilerPath: string): boolean {
10921092

10931093
/** CompilerPathAndArgs retains original casing of text input for compiler path and args */
10941094
export interface CompilerPathAndArgs {
1095-
compilerPath?: string | null;
1095+
compilerPath?: string;
10961096
compilerName: string;
10971097
compilerArgs?: string[];
10981098
compilerArgsFromCommandLineInPath: string[];
10991099
allCompilerArgs: string[];
1100+
error?: string;
1101+
telemetry?: { [key: string]: number };
11001102
}
11011103

1102-
export function extractCompilerPathAndArgs(useLegacyBehavior: boolean, inputCompilerPath?: string | null, compilerArgs?: string[]): CompilerPathAndArgs {
1103-
let compilerPath: string | undefined | null = inputCompilerPath;
1104+
/**
1105+
* Parse the compiler path input into a compiler path and compiler args. If there are no args in the input string, this function will have
1106+
* verified that the compiler exists. (e.g. `compilerArgsFromCommandLineInPath` will be empty)
1107+
*
1108+
* @param useLegacyBehavior - If true, use the legacy behavior of separating the compilerPath from the args.
1109+
* @param inputCompilerPath - The compiler path input from the user.
1110+
* @param compilerArgs - The compiler args input from the user.
1111+
* @param cwd - The directory used to resolve relative paths.
1112+
*/
1113+
export function extractCompilerPathAndArgs(useLegacyBehavior: boolean, inputCompilerPath?: string, compilerArgs?: string[], cwd?: string): CompilerPathAndArgs {
1114+
let compilerPath: string | undefined = inputCompilerPath;
11041115
let compilerName: string = "";
11051116
let compilerArgsFromCommandLineInPath: string[] = [];
1117+
const trimLegacyQuotes = (compilerPath?: string): string | undefined => {
1118+
if (compilerPath && useLegacyBehavior) {
1119+
// Try to trim quotes from compiler path.
1120+
const tempCompilerPath: string[] = extractArgs(compilerPath);
1121+
if (tempCompilerPath.length > 0) {
1122+
return tempCompilerPath[0];
1123+
}
1124+
}
1125+
return compilerPath;
1126+
};
11061127
if (compilerPath) {
11071128
compilerPath = compilerPath.trim();
11081129
if (isCl(compilerPath) || checkExecutableWithoutExtensionExistsSync(compilerPath)) {
11091130
// If the path ends with cl, or if a file is found at that path, accept it without further validation.
11101131
compilerName = path.basename(compilerPath);
1132+
} else if (cwd && checkExecutableWithoutExtensionExistsSync(path.join(cwd, compilerPath))) {
1133+
// If the path is relative and a file is found at that path, accept it without further validation.
1134+
compilerPath = path.join(cwd, compilerPath);
1135+
compilerName = path.basename(compilerPath);
11111136
} else if (compilerPath.startsWith("\"") || (os.platform() !== 'win32' && compilerPath.startsWith("'"))) {
11121137
// If the string starts with a quote, treat it as a command line.
11131138
// Otherwise, a path with a leading quote would not be valid.
1114-
if (useLegacyBehavior) {
1115-
compilerArgsFromCommandLineInPath = legacyExtractArgs(compilerPath);
1116-
if (compilerArgsFromCommandLineInPath.length > 0) {
1117-
compilerPath = compilerArgsFromCommandLineInPath.shift();
1118-
if (compilerPath) {
1119-
// Try to trim quotes from compiler path.
1120-
const tempCompilerPath: string[] | undefined = extractArgs(compilerPath);
1121-
if (tempCompilerPath && compilerPath.length > 0) {
1122-
compilerPath = tempCompilerPath[0];
1123-
}
1124-
compilerName = path.basename(compilerPath);
1125-
}
1126-
}
1127-
} else {
1128-
compilerArgsFromCommandLineInPath = extractArgs(compilerPath);
1129-
if (compilerArgsFromCommandLineInPath.length > 0) {
1130-
compilerPath = compilerArgsFromCommandLineInPath.shift();
1131-
if (compilerPath) {
1132-
compilerName = path.basename(compilerPath);
1133-
}
1134-
}
1139+
compilerArgsFromCommandLineInPath = useLegacyBehavior ? legacyExtractArgs(compilerPath) : extractArgs(compilerPath);
1140+
if (compilerArgsFromCommandLineInPath.length > 0) {
1141+
compilerPath = trimLegacyQuotes(compilerArgsFromCommandLineInPath.shift());
1142+
compilerName = path.basename(compilerPath ?? '');
11351143
}
11361144
} else {
1137-
const spaceStart: number = compilerPath.lastIndexOf(" ");
1138-
if (spaceStart !== -1) {
1139-
// There is no leading quote, but a space suggests it might be a command line.
1140-
// Try processing it as a command line, and validate that by checking for the executable.
1145+
if (compilerPath.includes(' ')) {
1146+
// There is no leading quote, but there is a space so we'll treat it as a command line.
11411147
const potentialArgs: string[] = useLegacyBehavior ? legacyExtractArgs(compilerPath) : extractArgs(compilerPath);
1142-
let potentialCompilerPath: string | undefined = potentialArgs.shift();
1143-
if (useLegacyBehavior) {
1144-
if (potentialCompilerPath) {
1145-
const tempCompilerPath: string[] | undefined = extractArgs(potentialCompilerPath);
1146-
if (tempCompilerPath && compilerPath.length > 0) {
1147-
potentialCompilerPath = tempCompilerPath[0];
1148-
}
1149-
}
1150-
}
1151-
if (potentialCompilerPath) {
1152-
if (isCl(potentialCompilerPath) || checkExecutableWithoutExtensionExistsSync(potentialCompilerPath)) {
1153-
compilerArgsFromCommandLineInPath = potentialArgs;
1154-
compilerPath = potentialCompilerPath;
1155-
compilerName = path.basename(compilerPath);
1156-
}
1157-
}
1148+
compilerPath = trimLegacyQuotes(potentialArgs.shift());
1149+
compilerArgsFromCommandLineInPath = potentialArgs;
11581150
}
1151+
compilerName = path.basename(compilerPath ?? '');
11591152
}
11601153
}
1161-
let allCompilerArgs: string[] = !compilerArgs ? [] : compilerArgs;
1162-
allCompilerArgs = allCompilerArgs.concat(compilerArgsFromCommandLineInPath);
1154+
const allCompilerArgs: string[] = (compilerArgs ?? []).concat(compilerArgsFromCommandLineInPath);
11631155
return { compilerPath, compilerName, compilerArgs, compilerArgsFromCommandLineInPath, allCompilerArgs };
11641156
}
11651157

Extension/test/scenarios/SimpleCppProject/assets/b i n/clang++

Whitespace-only changes.

Extension/test/scenarios/SimpleCppProject/assets/b i n/clang++.exe

Whitespace-only changes.

Extension/test/scenarios/SimpleCppProject/assets/bin/cl.exe

Whitespace-only changes.

Extension/test/scenarios/SimpleCppProject/assets/bin/clang-cl.exe

Whitespace-only changes.

Extension/test/scenarios/SimpleCppProject/assets/bin/gcc

Whitespace-only changes.

0 commit comments

Comments
 (0)