Skip to content

Commit be1fedc

Browse files
namrata111fnamrata111f
committed
Addressing null primary location handling
1 parent c6551f2 commit be1fedc

3 files changed

Lines changed: 145 additions & 11 deletions

File tree

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

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -62,11 +62,15 @@ const MESSAGE_CATALOG : MessageCatalog = {
6262
`-------------------------------------------`,
6363

6464
ConfigFieldDescription_suppressions:
65-
`Configuration for inline suppression markers in source code.\n` +
65+
`Configuration for inline and bulk suppressions.\n` +
6666
` disable_suppressions: Boolean to disable processing of suppression markers.\n` +
67+
` {file_path}: Array of bulk suppression rules for specific files or folders.\n` +
6768
`---- [Example usage]: ---------------------\n` +
6869
`suppressions:\n` +
6970
` disable_suppressions: false\n` +
71+
` "src/legacy.js":\n` +
72+
` - rule_selector: "eslint:no-console"\n` +
73+
` max_suppressed_violations: 5\n` +
7074
`-------------------------------------------`,
7175

7276
GenericEngineConfigOverview:

‎packages/code-analyzer-core/src/suppressions/bulk-suppression-processor.ts‎

Lines changed: 22 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -29,9 +29,14 @@ export type BulkSuppressionResult = {
2929

3030
/**
3131
* Applies bulk suppressions to violations based on config, updating quota tracking
32+
*
33+
* IMPORTANT: This function mutates the quotas Map as a side effect. The Map tracks
34+
* suppression counts across multiple calls and should be shared across all engines
35+
* in a single analysis run to maintain accurate quota limits.
36+
*
3237
* @param violations List of violations to process
3338
* @param bulkConfig Bulk suppression configuration from YAML
34-
* @param quotas Shared quota tracker (mutated by this function)
39+
* @param quotas Shared quota tracker (MUTATED by this function - maintains state across calls)
3540
* @param workspaceRoot Root directory for resolving relative paths
3641
* @returns Object containing unsuppressed violations and count of suppressions applied
3742
*/
@@ -47,24 +52,34 @@ export function applyBulkSuppressions(
4752

4853
// Sort violations for deterministic processing within this engine
4954
const sortedViolations = [...violations].sort((a, b) => {
50-
const fileA = a.getPrimaryLocation().getFile() || '';
51-
const fileB = b.getPrimaryLocation().getFile() || '';
55+
const primaryLocationA = a.getPrimaryLocation();
56+
const primaryLocationB = b.getPrimaryLocation();
57+
58+
const fileA = primaryLocationA?.getFile() || '';
59+
const fileB = primaryLocationB?.getFile() || '';
5260
if (fileA !== fileB) {
5361
return fileA.localeCompare(fileB);
5462
}
5563

56-
const lineA = a.getPrimaryLocation().getStartLine() || 0;
57-
const lineB = b.getPrimaryLocation().getStartLine() || 0;
64+
const lineA = primaryLocationA?.getStartLine() || 0;
65+
const lineB = primaryLocationB?.getStartLine() || 0;
5866
return lineA - lineB;
5967
});
6068

6169
const unsuppressedViolations: Violation[] = [];
6270
let suppressedCount = 0;
6371

6472
for (const violation of sortedViolations) {
65-
const violationFile = violation.getPrimaryLocation().getFile();
73+
const primaryLocation = violation.getPrimaryLocation();
74+
if (!primaryLocation) {
75+
// Skip violations with no primary location
76+
unsuppressedViolations.push(violation);
77+
continue;
78+
}
79+
80+
const violationFile = primaryLocation.getFile();
6681
if (!violationFile) {
67-
// Skip violations id no file path is present
82+
// Skip violations with no file path
6883
unsuppressedViolations.push(violation);
6984
continue;
7085
}

‎packages/code-analyzer-core/test/suppressions/bulk-suppression-processor.test.ts‎

Lines changed: 118 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -100,7 +100,7 @@ class MockViolation implements Violation {
100100
constructor(
101101
private rule: Rule,
102102
private message: string,
103-
private primaryLocation: CodeLocation
103+
private primaryLocation: CodeLocation | null
104104
) {}
105105

106106
getRule(): Rule {
@@ -112,10 +112,10 @@ class MockViolation implements Violation {
112112
}
113113

114114
getCodeLocations(): CodeLocation[] {
115-
return [this.primaryLocation];
115+
return this.primaryLocation ? [this.primaryLocation] : [];
116116
}
117117

118-
getPrimaryLocation(): CodeLocation {
118+
getPrimaryLocation(): CodeLocation | null {
119119
return this.primaryLocation;
120120
}
121121

@@ -811,4 +811,119 @@ describe('applyBulkSuppressions', () => {
811811
expect(result.suppressedCount).toBe(0);
812812
});
813813
});
814+
815+
describe('Null/undefined primary location handling', () => {
816+
it('should not suppress violations with null primary location', () => {
817+
const violations = [
818+
new MockViolation(
819+
new MockRule('pmd', 'UnusedMethod'),
820+
'Unused method',
821+
null // Null primary location
822+
),
823+
new MockViolation(
824+
new MockRule('pmd', 'UnusedMethod'),
825+
'Unused method',
826+
new MockCodeLocation(path.join(workspaceRoot, 'src', 'file1.apex'), 10, 1, 10, 20)
827+
)
828+
];
829+
830+
const bulkConfig = {
831+
'src/': [
832+
{
833+
rule_selector: 'pmd',
834+
max_suppressed_violations: null
835+
}
836+
]
837+
};
838+
839+
const quotas: BulkSuppressionQuotas = new Map();
840+
const result = applyBulkSuppressions(violations, bulkConfig, quotas, workspaceRoot);
841+
842+
// First violation should not be suppressed (null primary location)
843+
// Second violation should be suppressed (valid location)
844+
expect(result.unsuppressedViolations).toHaveLength(1);
845+
expect(result.suppressedCount).toBe(1);
846+
expect(result.unsuppressedViolations[0].getPrimaryLocation()).toBeNull();
847+
});
848+
849+
it('should not suppress violations with undefined file path', () => {
850+
const violations = [
851+
new MockViolation(
852+
new MockRule('pmd', 'UnusedMethod'),
853+
'Unused method',
854+
new MockCodeLocation(undefined, 10, 1, 10, 20) // No file path
855+
),
856+
new MockViolation(
857+
new MockRule('pmd', 'UnusedMethod'),
858+
'Unused method',
859+
new MockCodeLocation(path.join(workspaceRoot, 'src', 'file1.apex'), 10, 1, 10, 20)
860+
)
861+
];
862+
863+
const bulkConfig = {
864+
'src/': [
865+
{
866+
rule_selector: 'pmd',
867+
max_suppressed_violations: null
868+
}
869+
]
870+
};
871+
872+
const quotas: BulkSuppressionQuotas = new Map();
873+
const result = applyBulkSuppressions(violations, bulkConfig, quotas, workspaceRoot);
874+
875+
// First violation should not be suppressed (no file path)
876+
// Second violation should be suppressed (valid file path)
877+
expect(result.unsuppressedViolations).toHaveLength(1);
878+
expect(result.suppressedCount).toBe(1);
879+
expect(result.unsuppressedViolations[0].getPrimaryLocation()?.getFile()).toBeUndefined();
880+
});
881+
882+
it('should handle sorting with null primary locations', () => {
883+
const violations = [
884+
new MockViolation(
885+
new MockRule('pmd', 'UnusedMethod'),
886+
'Unused method 3',
887+
new MockCodeLocation(path.join(workspaceRoot, 'src', 'file2.apex'), 5, 1, 5, 20)
888+
),
889+
new MockViolation(
890+
new MockRule('pmd', 'UnusedMethod'),
891+
'Unused method 1',
892+
null // Null primary location
893+
),
894+
new MockViolation(
895+
new MockRule('pmd', 'UnusedMethod'),
896+
'Unused method 2',
897+
new MockCodeLocation(path.join(workspaceRoot, 'src', 'file1.apex'), 10, 1, 10, 20)
898+
),
899+
new MockViolation(
900+
new MockRule('pmd', 'UnusedMethod'),
901+
'Unused method 4',
902+
new MockCodeLocation(undefined, 10, 1, 10, 20) // No file path
903+
)
904+
];
905+
906+
const bulkConfig = {
907+
'src/': [
908+
{
909+
rule_selector: 'pmd',
910+
max_suppressed_violations: null
911+
}
912+
]
913+
};
914+
915+
const quotas: BulkSuppressionQuotas = new Map();
916+
const result = applyBulkSuppressions(violations, bulkConfig, quotas, workspaceRoot);
917+
918+
// Violations with null location or undefined file should not be suppressed
919+
// Violations with valid locations should be suppressed
920+
expect(result.unsuppressedViolations).toHaveLength(2);
921+
expect(result.suppressedCount).toBe(2);
922+
923+
// Verify the unsuppressed violations are the ones with null/undefined locations
924+
const unsuppressedMessages = result.unsuppressedViolations.map(v => v.getMessage());
925+
expect(unsuppressedMessages).toContain('Unused method 1'); // Null primary location
926+
expect(unsuppressedMessages).toContain('Unused method 4'); // Undefined file
927+
});
928+
});
814929
});

0 commit comments

Comments
 (0)