Skip to content

Commit 060d0c7

Browse files
nikhil-mittal-165nikhil-mittal-165
committed
Merge feature/fixes-suggestion-data-modelling into feature/eslint-fixes-suggestions
2 parents 4b4b262 + a8b63e2 commit 060d0c7

19 files changed

Lines changed: 674 additions & 63 deletions

File tree

‎packages/code-analyzer-core/src/code-analyzer.ts‎

Lines changed: 56 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -763,6 +763,8 @@ function validateEngineRunResults(engineName: string, apiEngineRunResults: engAp
763763
validateViolationRuleName(violation, engineName, ruleSelection);
764764
validateViolationCodeLocations(violation, engineName);
765765
validateViolationPrimaryLocationIndex(violation, engineName);
766+
validateFixCodeLocations(violation, engineName);
767+
validateSuggestionCodeLocations(violation, engineName);
766768
}
767769
}
768770

@@ -786,47 +788,68 @@ function validateViolationCodeLocations(violation: engApi.Violation, engineName:
786788
throw new Error(getMessage('EngineReturnedViolationWithEmptyCodeLocationArray', engineName, violation.ruleName));
787789
}
788790
for (const codeLocation of violation.codeLocations) {
789-
const absFile: string = toAbsolutePath(codeLocation.file);
790-
fs.existsSync(absFile)
791+
validateCodeLocation(codeLocation, engineName, violation.ruleName);
792+
}
793+
}
791794

792-
if (!fs.existsSync(absFile)) {
793-
throw new Error(getMessage('EngineReturnedViolationWithCodeLocationFileThatDoesNotExist',
794-
engineName, violation.ruleName, absFile));
795-
}
795+
function validateFixCodeLocations(violation: engApi.Violation, engineName: string): void {
796+
if (!violation.fixes) {
797+
return;
798+
}
799+
for (const fix of violation.fixes) {
800+
validateCodeLocation(fix.location, engineName, violation.ruleName);
801+
}
802+
}
796803

797-
if (!fs.statSync(absFile).isFile()) {
798-
throw new Error(getMessage('EngineReturnedViolationWithCodeLocationFileAsFolder',
799-
engineName, violation.ruleName, absFile));
800-
}
804+
function validateSuggestionCodeLocations(violation: engApi.Violation, engineName: string): void {
805+
if (!violation.suggestions) {
806+
return;
807+
}
808+
for (const suggestion of violation.suggestions) {
809+
validateCodeLocation(suggestion.location, engineName, violation.ruleName);
810+
}
811+
}
801812

802-
if (!isValidLineOrColumn(codeLocation.startLine)) {
803-
throw new Error(getMessage('EngineReturnedViolationWithCodeLocationWithInvalidLineOrColumn',
804-
engineName, violation.ruleName, 'startLine', codeLocation.startLine));
805-
}
813+
function validateCodeLocation(codeLocation: engApi.CodeLocation, engineName: string, ruleName: string): void {
814+
const absFile: string = toAbsolutePath(codeLocation.file);
806815

807-
if (!isValidLineOrColumn(codeLocation.startColumn)) {
816+
if (!fs.existsSync(absFile)) {
817+
throw new Error(getMessage('EngineReturnedViolationWithCodeLocationFileThatDoesNotExist',
818+
engineName, ruleName, absFile));
819+
}
820+
821+
if (!fs.statSync(absFile).isFile()) {
822+
throw new Error(getMessage('EngineReturnedViolationWithCodeLocationFileAsFolder',
823+
engineName, ruleName, absFile));
824+
}
825+
826+
if (!isValidLineOrColumn(codeLocation.startLine)) {
827+
throw new Error(getMessage('EngineReturnedViolationWithCodeLocationWithInvalidLineOrColumn',
828+
engineName, ruleName, 'startLine', codeLocation.startLine));
829+
}
830+
831+
if (!isValidLineOrColumn(codeLocation.startColumn)) {
832+
throw new Error(getMessage('EngineReturnedViolationWithCodeLocationWithInvalidLineOrColumn',
833+
engineName, ruleName, 'startColumn', codeLocation.startColumn));
834+
}
835+
836+
if (codeLocation.endLine !== undefined) {
837+
if (!isValidLineOrColumn(codeLocation.endLine)) {
808838
throw new Error(getMessage('EngineReturnedViolationWithCodeLocationWithInvalidLineOrColumn',
809-
engineName, violation.ruleName, 'startColumn', codeLocation.startColumn));
839+
engineName, ruleName, 'endLine', codeLocation.endLine));
840+
} else if (codeLocation.endLine < codeLocation.startLine) {
841+
throw new Error(getMessage('EngineReturnedViolationWithCodeLocationWithEndLineBeforeStartLine',
842+
engineName, ruleName, codeLocation.endLine, codeLocation.startLine));
810843
}
811844

812-
if (codeLocation.endLine !== undefined) {
813-
if (!isValidLineOrColumn(codeLocation.endLine)) {
845+
// istanbul ignore else
846+
if (codeLocation.endColumn !== undefined) {
847+
if (!isValidLineOrColumn(codeLocation.endColumn)) {
814848
throw new Error(getMessage('EngineReturnedViolationWithCodeLocationWithInvalidLineOrColumn',
815-
engineName, violation.ruleName, 'endLine', codeLocation.endLine));
816-
} else if (codeLocation.endLine < codeLocation.startLine) {
817-
throw new Error(getMessage('EngineReturnedViolationWithCodeLocationWithEndLineBeforeStartLine',
818-
engineName, violation.ruleName, codeLocation.endLine, codeLocation.startLine));
819-
}
820-
821-
// istanbul ignore else
822-
if (codeLocation.endColumn !== undefined) {
823-
if (!isValidLineOrColumn(codeLocation.endColumn)) {
824-
throw new Error(getMessage('EngineReturnedViolationWithCodeLocationWithInvalidLineOrColumn',
825-
engineName, violation.ruleName, 'endColumn', codeLocation.endColumn));
826-
} else if (codeLocation.endLine == codeLocation.startLine && codeLocation.endColumn < codeLocation.startColumn) {
827-
throw new Error(getMessage('EngineReturnedViolationWithCodeLocationWithEndColumnBeforeStartColumnOnSameLine',
828-
engineName, violation.ruleName, codeLocation.endColumn, codeLocation.startColumn));
829-
}
849+
engineName, ruleName, 'endColumn', codeLocation.endColumn));
850+
} else if (codeLocation.endLine == codeLocation.startLine && codeLocation.endColumn < codeLocation.startColumn) {
851+
throw new Error(getMessage('EngineReturnedViolationWithCodeLocationWithEndColumnBeforeStartColumnOnSameLine',
852+
engineName, ruleName, codeLocation.endColumn, codeLocation.startColumn));
830853
}
831854
}
832855
}

‎packages/code-analyzer-core/test/code-analyzer.test.ts‎

Lines changed: 111 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -686,6 +686,117 @@ describe("Tests for the run method of CodeAnalyzer", () => {
686686
expect(violations[0].getCodeLocations()[0].getEndColumn()).toBeUndefined();
687687
});
688688

689+
it("When an engine returns a fix with a code location file that does not exist, then an error is thrown", async () => {
690+
const badViolation: engApi.Violation = stubs.getSampleViolationWithFixes();
691+
badViolation.fixes![0].location.file = 'test/doesNotExist';
692+
stubEngine1.resultsToReturn = { violations: [badViolation] };
693+
await expect(codeAnalyzer.run(selection, sampleRunOptions)).rejects.toThrow(
694+
getMessage('EngineReturnedViolationWithCodeLocationFileThatDoesNotExist',
695+
'stubEngine1', 'stub1RuleA', path.resolve('test', 'doesNotExist')));
696+
});
697+
698+
it("When an engine returns a fix with an invalid startLine, then an error is thrown", async () => {
699+
const badViolation: engApi.Violation = stubs.getSampleViolationWithFixes();
700+
badViolation.fixes![0].location.startLine = -1;
701+
stubEngine1.resultsToReturn = { violations: [badViolation] };
702+
await expect(codeAnalyzer.run(selection, sampleRunOptions)).rejects.toThrow(
703+
getMessage('EngineReturnedViolationWithCodeLocationWithInvalidLineOrColumn',
704+
'stubEngine1', 'stub1RuleA', 'startLine', -1));
705+
});
706+
707+
it("When an engine returns a fix with endLine before startLine, then an error is thrown", async () => {
708+
const badViolation: engApi.Violation = stubs.getSampleViolationWithFixes();
709+
badViolation.fixes![0].location.startLine = 10;
710+
badViolation.fixes![0].location.endLine = 5;
711+
stubEngine1.resultsToReturn = { violations: [badViolation] };
712+
await expect(codeAnalyzer.run(selection, sampleRunOptions)).rejects.toThrow(
713+
getMessage('EngineReturnedViolationWithCodeLocationWithEndLineBeforeStartLine',
714+
'stubEngine1', 'stub1RuleA', 5, 10));
715+
});
716+
717+
it("When an engine returns a suggestion with a code location file that does not exist, then an error is thrown", async () => {
718+
const badViolation: engApi.Violation = stubs.getSampleViolationWithSuggestions();
719+
badViolation.suggestions![0].location.file = 'test/doesNotExist';
720+
stubEngine1.resultsToReturn = { violations: [badViolation] };
721+
await expect(codeAnalyzer.run(selection, sampleRunOptions)).rejects.toThrow(
722+
getMessage('EngineReturnedViolationWithCodeLocationFileThatDoesNotExist',
723+
'stubEngine1', 'stub1RuleA', path.resolve('test', 'doesNotExist')));
724+
});
725+
726+
it("When an engine returns a suggestion with an invalid startColumn, then an error is thrown", async () => {
727+
const badViolation: engApi.Violation = stubs.getSampleViolationWithSuggestions();
728+
badViolation.suggestions![0].location.startColumn = 0;
729+
stubEngine1.resultsToReturn = { violations: [badViolation] };
730+
await expect(codeAnalyzer.run(selection, sampleRunOptions)).rejects.toThrow(
731+
getMessage('EngineReturnedViolationWithCodeLocationWithInvalidLineOrColumn',
732+
'stubEngine1', 'stub1RuleA', 'startColumn', 0));
733+
});
734+
735+
it("When an engine returns a violation with fixes, then getFixes returns the correct data", async () => {
736+
stubEngine1.resultsToReturn = { violations: [stubs.getSampleViolationWithFixes()] };
737+
const overallResults: RunResults = await codeAnalyzer.run(selection, sampleRunOptions);
738+
const violations: Violation[] = overallResults.getViolations();
739+
expect(violations).toHaveLength(1);
740+
741+
const fixes = violations[0].getFixes();
742+
expect(fixes).toHaveLength(1);
743+
expect(fixes[0].getFixedCode()).toEqual('const correctedValue = true;');
744+
expect(fixes[0].getLocation().getFile()).toEqual(path.resolve('test/config.test.ts'));
745+
expect(fixes[0].getLocation().getStartLine()).toEqual(3);
746+
expect(fixes[0].getLocation().getStartColumn()).toEqual(6);
747+
expect(fixes[0].getLocation().getEndLine()).toEqual(3);
748+
expect(fixes[0].getLocation().getEndColumn()).toEqual(20);
749+
});
750+
751+
it("When an engine returns a violation with suggestions, then getSuggestions returns the correct data", async () => {
752+
stubEngine1.resultsToReturn = { violations: [stubs.getSampleViolationWithSuggestions()] };
753+
const overallResults: RunResults = await codeAnalyzer.run(selection, sampleRunOptions);
754+
const violations: Violation[] = overallResults.getViolations();
755+
expect(violations).toHaveLength(1);
756+
757+
const suggestions = violations[0].getSuggestions();
758+
expect(suggestions).toHaveLength(2);
759+
expect(suggestions[0].getMessage()).toEqual('Consider using a boolean literal instead');
760+
expect(suggestions[0].getLocation().getFile()).toEqual(path.resolve('test/config.test.ts'));
761+
expect(suggestions[1].getMessage()).toEqual('Consider removing this unused variable');
762+
});
763+
764+
it("When an engine returns a violation with both fixes and suggestions, then both are accessible", async () => {
765+
stubEngine1.resultsToReturn = { violations: [stubs.getSampleViolationWithFixesAndSuggestions()] };
766+
const overallResults: RunResults = await codeAnalyzer.run(selection, sampleRunOptions);
767+
const violations: Violation[] = overallResults.getViolations();
768+
expect(violations).toHaveLength(1);
769+
expect(violations[0].getFixes()).toHaveLength(2);
770+
expect(violations[0].getSuggestions()).toHaveLength(1);
771+
});
772+
773+
it("When an engine returns a violation without fixes or suggestions, then getFixes and getSuggestions return empty arrays", async () => {
774+
stubEngine1.resultsToReturn = { violations: [stubs.getSampleViolationForStub1RuleA()] };
775+
const overallResults: RunResults = await codeAnalyzer.run(selection, sampleRunOptions);
776+
const violations: Violation[] = overallResults.getViolations();
777+
expect(violations).toHaveLength(1);
778+
expect(violations[0].getFixes()).toEqual([]);
779+
expect(violations[0].getSuggestions()).toEqual([]);
780+
});
781+
782+
it("When includeFixes and includeSuggestions are specified in RunOptions, they are forwarded to the engine", async () => {
783+
stubEngine1.resultsToReturn = { violations: [stubs.getSampleViolationForStub1RuleA()] };
784+
await codeAnalyzer.run(selection, {
785+
...sampleRunOptions,
786+
includeFixes: true,
787+
includeSuggestions: true
788+
});
789+
expect(stubEngine1.runRulesCallHistory[0].runOptions.includeFixes).toBe(true);
790+
expect(stubEngine1.runRulesCallHistory[0].runOptions.includeSuggestions).toBe(true);
791+
});
792+
793+
it("When includeFixes and includeSuggestions are not specified in RunOptions, they are undefined in the engine run options", async () => {
794+
stubEngine1.resultsToReturn = { violations: [stubs.getSampleViolationForStub1RuleA()] };
795+
await codeAnalyzer.run(selection, sampleRunOptions);
796+
expect(stubEngine1.runRulesCallHistory[0].runOptions.includeFixes).toBeUndefined();
797+
expect(stubEngine1.runRulesCallHistory[0].runOptions.includeSuggestions).toBeUndefined();
798+
});
799+
689800
it("When an engine throws an exception when running, then a result is returned with a Critical violation of type UnexpectedError", async () => {
690801
codeAnalyzer = createCodeAnalyzer();
691802
await codeAnalyzer.addEnginePlugin(new stubs.ThrowingEnginePlugin());

‎packages/code-analyzer-core/test/output-format.test.ts‎

Lines changed: 104 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -174,6 +174,110 @@ describe("RunResultsFormatter Tests", () => {
174174
});
175175
});
176176

177+
describe("Output format tests for fixes and suggestions", () => {
178+
let resultsWithFixes: RunResults;
179+
180+
beforeAll(async () => {
181+
const codeAnalyzer: CodeAnalyzer = new CodeAnalyzer(CodeAnalyzerConfig.withDefaults());
182+
codeAnalyzer._setClock(new FixedClock(new Date(2024, 6, 3, 9, 14, 34, 567)));
183+
const stubPlugin: stubs.StubEnginePlugin = new stubs.StubEnginePlugin();
184+
await codeAnalyzer.addEnginePlugin(stubPlugin);
185+
(stubPlugin.getCreatedEngine('stubEngine1') as stubs.StubEngine1).resultsToReturn = {
186+
violations: [
187+
stubs.getSampleViolationWithFixes(),
188+
stubs.getSampleViolationWithSuggestions(),
189+
stubs.getSampleViolationWithFixesAndSuggestions()
190+
]
191+
};
192+
const selection = await codeAnalyzer.selectRules(['all']);
193+
resultsWithFixes = await codeAnalyzer.run(selection, {workspace: await codeAnalyzer.createWorkspace(['test'])});
194+
});
195+
196+
describe("JSON output format with fixes and suggestions", () => {
197+
it("Violations with fixes include fixes array in JSON output", () => {
198+
const json = JSON.parse(resultsWithFixes.toFormattedOutput(OutputFormat.JSON));
199+
const violationsWithFixes = json.violations.filter((v: {fixes?: unknown[]}) => v.fixes && v.fixes.length > 0);
200+
expect(violationsWithFixes.length).toBeGreaterThanOrEqual(1);
201+
202+
const fix = violationsWithFixes[0].fixes[0];
203+
expect(fix).toHaveProperty('location');
204+
expect(fix).toHaveProperty('fixedCode');
205+
expect(fix.location).toHaveProperty('file');
206+
expect(fix.location).toHaveProperty('startLine');
207+
expect(fix.location).toHaveProperty('startColumn');
208+
});
209+
210+
it("Violations with suggestions include suggestions array in JSON output", () => {
211+
const json = JSON.parse(resultsWithFixes.toFormattedOutput(OutputFormat.JSON));
212+
const violationsWithSuggestions = json.violations.filter((v: {suggestions?: unknown[]}) => v.suggestions && v.suggestions.length > 0);
213+
expect(violationsWithSuggestions.length).toBeGreaterThanOrEqual(1);
214+
215+
const suggestion = violationsWithSuggestions[0].suggestions[0];
216+
expect(suggestion).toHaveProperty('location');
217+
expect(suggestion).toHaveProperty('message');
218+
});
219+
220+
it("Violations without fixes do not have a fixes key in JSON output", () => {
221+
const json = JSON.parse(resultsWithFixes.toFormattedOutput(OutputFormat.JSON));
222+
const violationsWithoutFixes = json.violations.filter((v: {fixes?: unknown[]}) => !v.fixes);
223+
expect(violationsWithoutFixes.length).toBeGreaterThanOrEqual(1);
224+
expect(violationsWithoutFixes[0]).not.toHaveProperty('fixes');
225+
});
226+
227+
it("Fix location file paths are relative to runDir in JSON output", () => {
228+
const json = JSON.parse(resultsWithFixes.toFormattedOutput(OutputFormat.JSON));
229+
const violationWithFix = json.violations.find((v: {fixes?: unknown[]}) => v.fixes && v.fixes.length > 0);
230+
const fixFile = violationWithFix.fixes[0].location.file;
231+
expect(fixFile).not.toContain(json.runDir);
232+
expect(path.isAbsolute(fixFile)).toBe(false);
233+
});
234+
});
235+
236+
describe("XML output format with fixes and suggestions", () => {
237+
it("Violations with fixes include fix nodes in XML output", () => {
238+
const xml = resultsWithFixes.toFormattedOutput(OutputFormat.XML);
239+
expect(xml).toContain('<fixes>');
240+
expect(xml).toContain('<fix>');
241+
expect(xml).toContain('<fixedCode>');
242+
});
243+
244+
it("Violations with suggestions include suggestion nodes in XML output", () => {
245+
const xml = resultsWithFixes.toFormattedOutput(OutputFormat.XML);
246+
expect(xml).toContain('<suggestions>');
247+
expect(xml).toContain('<suggestion>');
248+
expect(xml).toContain('<message>');
249+
});
250+
});
251+
252+
describe("SARIF output format with fixes", () => {
253+
it("Violations with fixes include fix data in SARIF output", () => {
254+
const sarif = JSON.parse(resultsWithFixes.toFormattedOutput(OutputFormat.SARIF));
255+
const allResults = sarif.runs.flatMap((run: {results: unknown[]}) => run.results);
256+
const resultsWithFixData = allResults.filter((r: {fixes?: unknown[]}) => r.fixes && r.fixes.length > 0);
257+
expect(resultsWithFixData.length).toBeGreaterThanOrEqual(1);
258+
259+
const sarifFix = resultsWithFixData[0].fixes[0];
260+
expect(sarifFix).toHaveProperty('artifactChanges');
261+
expect(sarifFix.artifactChanges[0]).toHaveProperty('replacements');
262+
expect(sarifFix.artifactChanges[0].replacements[0]).toHaveProperty('deletedRegion');
263+
expect(sarifFix.artifactChanges[0].replacements[0]).toHaveProperty('insertedContent');
264+
});
265+
266+
it("Suggestions are not included in SARIF output", () => {
267+
const sarifStr = resultsWithFixes.toFormattedOutput(OutputFormat.SARIF);
268+
expect(sarifStr).not.toContain('"suggestions"');
269+
});
270+
});
271+
272+
describe("CSV output format with fixes and suggestions", () => {
273+
it("Fixes and suggestions are not included in CSV output", () => {
274+
const csv = resultsWithFixes.toFormattedOutput(OutputFormat.CSV);
275+
expect(csv).not.toContain('fixedCode');
276+
expect(csv).not.toContain('const correctedValue');
277+
});
278+
});
279+
});
280+
177281
describe("RuleSelectionFormatter Tests", () => {
178282

179283
describe("Tests for the JSON output format", () => {

0 commit comments

Comments
 (0)