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.

Sign up for GitHub

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(cli): account for parse errors being of string type #444

Merged
merged 3 commits into from
Feb 22, 2025
Merged

fix(cli): account for parse errors being of string type #444

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
630af38
fix(cli): account for parse errors being of string type
joshzcold Feb 21, 2025
997dbe8
chore(cli): add comment about the text we are parsing
joshzcold Feb 21, 2025
5596e28
test(api, cli): fix error count numbers for tests after fixing parseE…
joshzcold Feb 21, 2025
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
43 changes: 32 additions & 11 deletions lib/codenarc-factory.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -241,12 +241,30 @@ async function parseCodeNarcResult(options, codeNarcBaseDir, codeNarcJsonResult,
if (files[fileNm] == null) {
files[fileNm] = { errors: [] };
}
for (const parseError of fileParseErrors) {
for (const parseMessage of fileParseErrors) {
// Convert GroovyShell.parse Compilation exception error into NpmGroovyLint exception
let msg =
parseError.cause && parseError.cause.message ? parseError.cause.message : `Unknown parsing error: ${JSON.stringify(parseError)}`;

let parseError = {
cause: {
message: null
}
}
/* NOTE parsing messages like this
'/path/to/Pipeline.groovy: 2: unable to resolve class org.sm.Something\n' +
' @ line 2, column 1.\n' +
' import org.sm.Something\n' +
' ^\n',
*/
const parts = /(.*):\s(\d+):\s(.*)/s.exec(parseMessage)
if(!parts || parts.length < 4) {
parseError.cause.message = `Unknown parsing error: ${JSON.stringify(parseMessage)}`;
} else {
parseError.cause.line = parts[2]
parseError.cause.message = parts[3]
}

// Remove 'unable to resolve class' error as GroovyShell.parse is called without ClassPath
if (msg.startsWith("unable to resolve class ")) {
if (parseError.cause.message.includes("unable to resolve class ")) {
continue;
}
// Create new error
Expand All @@ -255,15 +273,18 @@ async function parseCodeNarcResult(options, codeNarcBaseDir, codeNarcJsonResult,
line: parseError.cause && parseError.cause.line ? parseError.cause.line : 0,
rule: "NglParseError",
severity: "error",
msg: msg,
msg: parseError.cause.message,
};
// TODO would need a more complicated regex to parse for a "range"
// might not be really needed in this case for npm-groovy-lint
//
// Add range if provided
if (parseError.cause && parseError.cause.startColumn) {
errItemParse.range = {
start: { line: parseError.cause.startLine, character: parseError.cause.startColumn },
end: { line: parseError.cause.endLine, character: parseError.cause.endColumn },
};
}
//if (parseError.cause && parseError.cause.startColumn) {
// errItemParse.range = {
// start: { line: parseError.cause.startLine, character: parseError.cause.startColumn },
// end: { line: parseError.cause.endLine, character: parseError.cause.endColumn },
// };
//}

files[fileNm].errors.push(errItemParse);
errId++;
Expand Down
6 changes: 3 additions & 3 deletions test/lint-api.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -316,7 +316,7 @@ describe("Lint with API", () => {
assert(linter.status === 1, `Linter status is 1 (${linter.status} returned)`);
assert(linter.outputString.includes("warning"), "Output string contains warning");
assert(Object.keys(linter.lintResult.files).length === 2, "Files array contains 2 files");
assert(linter.lintResult.summary.totalFoundErrorNumber === 5, "Error found");
assert(linter.lintResult.summary.totalFoundErrorNumber === 4, "Error found");
assert(linter.lintResult.summary.totalFoundWarningNumber === 6, "Warnings found");
assert(linter.lintResult.summary.totalFoundInfoNumber === 65, "Infos found");
checkCodeNarcCallsCounter(1);
Expand Down Expand Up @@ -354,7 +354,7 @@ describe("Lint with API", () => {
assert(linter.status === 1, `Linter status is 1 (${linter.status} returned)`);
assert(linter.outputString.includes("warning"), "Output string contains warning");
assert(Object.keys(linter.lintResult.files).length === 2, "Files array contains 2 files");
assert(linter.lintResult.summary.totalFoundErrorNumber === 5, "Error found");
assert(linter.lintResult.summary.totalFoundErrorNumber === 4, "Error found");
assert(linter.lintResult.summary.totalFoundWarningNumber === 6, "Warnings found");
assert(linter.lintResult.summary.totalFoundInfoNumber === 65, "Infos found");
checkCodeNarcCallsCounter(1);
Expand All @@ -372,7 +372,7 @@ describe("Lint with API", () => {
assert(linter.status === 1, `Linter status is 1 (${linter.status} returned)`);
assert(linter.outputString.includes("warning"), "Output string contains warning");
assert(Object.keys(linter.lintResult.files).length === 12, `Expected 2 files got ${Object.keys(linter.lintResult.files).length}`);
assert(linter.lintResult.summary.totalFoundErrorNumber === 19, `Expected 19 errors to ${linter.lintResult.summary.totalFoundErrorNumber}`);
assert(linter.lintResult.summary.totalFoundErrorNumber === 12, `Expected 12 errors to ${linter.lintResult.summary.totalFoundErrorNumber}`);
assert(linter.lintResult.summary.totalFoundWarningNumber === 333, `Expected 333 warnings to ${linter.lintResult.summary.totalFoundWarningNumber}`);
assert(linter.lintResult.summary.totalFoundInfoNumber === 1649, `Expected 1649 infos to ${linter.lintResult.summary.totalFoundInfoNumber}`);
checkCodeNarcCallsCounter(1);
Expand Down
Loading