Skip to content

Commit

Permalink
feat(checkstyleconfig): adds a removeDuplicates option to remove du…
Browse files Browse the repository at this point in the history
…plicate violations

This creates a new optional `removeDuplicates` option that will filter dupicate violations found
while scanning and report them just once.

fix #2
  • Loading branch information
jmartinesp authored and Rouby committed May 17, 2023
1 parent 38f5b73 commit 63afe10
Show file tree
Hide file tree
Showing 4 changed files with 79 additions and 6 deletions.
5 changes: 5 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,11 @@ interface CheckstyleConfig {
* Optional: Override the violation formatter to customize the output message.
*/
violationFormatter?: ViolationFormatter

/**
* Optional: If set to true, it will remove duplicate violations.
*/
removeDuplicates?: boolean
}
```
## Changelog
Expand Down
5 changes: 5 additions & 0 deletions src/checkstyleconfig.ts
Original file line number Diff line number Diff line change
Expand Up @@ -30,4 +30,9 @@ interface CheckstyleConfig {
* Optional: Sets the root directory of the project. Defaults to the current working directory.
*/
projectRoot?: string

/**
* Optional: If set to true, it will remove duplicate violations.
*/
removeDuplicates?: boolean
}
48 changes: 48 additions & 0 deletions src/index.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -168,6 +168,54 @@ describe("scan()", () => {
})


it(`removes duplicate violations if option is enabled`, async () => {
const git = {
modified_files: ["feature/src/main/res/layout/fragment_password_reset.xml"],
created_files: [],
structuredDiffForFile: async () => ({ chunks: [{ changes: [{ type: "add", ln: 13 }] }] }),
}
let counter = 0
let highMark = 0
global.danger = { git }
global.warn = jest.fn(() => ++counter)

mockGlob.mockImplementation(() =>
Array.from({ length: 123 }, () => "feature/src/main/res/layout/fragment_password_reset.xml"),
)
mockFileSync.mockReset().mockImplementation((path: string) => {
return `<?xml version="1.0" encoding="UTF-8"?>
<issues format="5" by="lint 4.2.0-alpha01">
<issue
id="HardcodedText"
severity="Warning"
message="Hardcoded string &quot;Email Address&quot;, should use \`@string\` resource"
category="Internationalization"
priority="5"
summary="Hardcoded text"
explanation="Hardcoding text attributes directly in layout files is bad for several reasons:&#xA;&#xA;* When creating configuration variations (for example for landscape or portrait) you have to repeat the actual text (and keep it up to date when making changes)&#xA;&#xA;* The application cannot be translated to other languages by just adding new translations for existing string resources.&#xA;&#xA;There are quickfixes to automatically extract this hardcoded string into a resource lookup."
errorLine1=" android:hint=&quot;Email Address&quot;"
errorLine2=" ~~~~~~~~~~~~~~~~~~~~~~~~~~~~">
<location
file="${root}/feature/src/main/res/layout/fragment_password_reset.xml"
line="13"
column="9"/>
</issue>
</issues>`
})

await scan({
fileMask: "",
reportSeverity: true,
requireLineModification: true,
projectRoot: root,
removeDuplicates: true,
})

expect(counter).toEqual(1)
})

it(`scans maximum ${maxParallel} files in parallel to prevent OoM exceptions`, async () => {
const git = {
modified_files: ["feature/src/main/res/layout/fragment_password_reset.xml"],
Expand Down
27 changes: 21 additions & 6 deletions src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,8 @@ export async function scan(config: CheckstyleConfig) {
const root = config.projectRoot ?? process.cwd()
const git = danger.git

let accumulated: { [id: string] : Violation; } = {}

const files: string[] = await new Promise((resolve, reject) =>
glob(config.fileMask, (err, result) => (err ? reject(err) : resolve(result))),
)
Expand All @@ -36,19 +38,32 @@ export async function scan(config: CheckstyleConfig) {
await scanXmlReport(git, xmlReport, root, config.requireLineModification, (violation: Violation) => {
var severity = violation.severity
if (!config.reportSeverity) {
severity = "info"
violation.severity = "info"
}

var msg = violationFormatter.format(violation)
if (config.outputPrefix) {
msg = config.outputPrefix + msg
if (config.removeDuplicates) {
let id = `${ violation.issueId }_${ violation.file }:${ violation.line }.${ violation.column }`
accumulated[id] = violation
} else {
generateMessageAndReport(violation, violationFormatter, config.outputPrefix)
}

sendViolationBySeverity(msg, violation.file, violation.line, violation.severity)
})
}),
)
}

for (let id in accumulated) {
let violation = accumulated[id]
generateMessageAndReport(violation, violationFormatter, config.outputPrefix)
}
}

function generateMessageAndReport(violation: Violation, violationFormatter: ViolationFormatter, outputPrefix?: string) {
var msg = violationFormatter.format(violation)
if (outputPrefix) {
msg = outputPrefix + msg
}
sendViolationBySeverity(msg, violation.file, violation.line, violation.severity)
}

export async function scanXmlReport(
Expand Down

0 comments on commit 63afe10

Please sign in to comment.