From 63afe101b6bd3f2d89209181bd7de6259e497173 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Jorge=20Mart=C3=ADn?= <jorgem@element.io>
Date: Thu, 4 May 2023 14:51:09 +0200
Subject: [PATCH] feat(checkstyleconfig): adds a `removeDuplicates` option to
 remove duplicate violations

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

fix #2
---
 README.md               |  5 +++++
 src/checkstyleconfig.ts |  5 +++++
 src/index.test.ts       | 48 +++++++++++++++++++++++++++++++++++++++++
 src/index.ts            | 27 +++++++++++++++++------
 4 files changed, 79 insertions(+), 6 deletions(-)

diff --git a/README.md b/README.md
index cd1105c..66ae378 100644
--- a/README.md
+++ b/README.md
@@ -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
diff --git a/src/checkstyleconfig.ts b/src/checkstyleconfig.ts
index 71c8376..fe95843 100644
--- a/src/checkstyleconfig.ts
+++ b/src/checkstyleconfig.ts
@@ -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
 }
diff --git a/src/index.test.ts b/src/index.test.ts
index b53cd19..8ba3d41 100644
--- a/src/index.test.ts
+++ b/src/index.test.ts
@@ -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"],
diff --git a/src/index.ts b/src/index.ts
index 1a34802..bce97b8 100644
--- a/src/index.ts
+++ b/src/index.ts
@@ -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))),
   )
@@ -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(