Markdown as Violation as well

source/commands/utils/reporting.ts

@@ -11,7 +11,7 @@ export const resultsWithFailure = (failure: string, moreMarkdown?: string): Dang
     warnings: [],
     messages: [],
     fails: [fail],
-    markdowns: moreMarkdown ? [moreMarkdown] : [],
+    markdowns: moreMarkdown ? [{ message: moreMarkdown }] : [],
   }
 }
 

source/dsl/DangerResults.ts

@@ -1,5 +1,4 @@
-import { Violation } from "../dsl/Violation"
-import { MarkdownString } from "../dsl/Aliases"
+import { Violation, isInline } from "../dsl/Violation"
 
 /**
  * The representation of what running a Dangerfile generates.
@@ -26,7 +25,7 @@ export interface DangerResults {
   /**
    * Markdown messages to attach at the bottom of the comment
    */
-  markdowns: MarkdownString[]
+  markdowns: Violation[]
 }
 
 export interface DangerRuntimeContainer extends DangerResults {
@@ -35,3 +34,122 @@ export interface DangerRuntimeContainer extends DangerResults {
    */
   scheduled?: any[]
 }
+
+export interface DangerInlineResults {
+  /**
+   * Path to the file
+   *
+   * @type {string}
+   */
+  file: string
+
+  /**
+   * Line in the file
+   *
+   * @type {string}
+   */
+
+  line: number
+
+  /**
+   * Violation results for given file/line
+   *
+   * @type {results}
+   */
+  results: DangerResults
+}
+
+export const emptyDangerResults = {
+  fails: [],
+  warnings: [],
+  messages: [],
+  markdowns: [],
+}
+
+export function inlineResults(results: DangerResults): DangerResults {
+  return sortResults({
+    fails: results.fails.filter(m => isInline(m)),
+    warnings: results.warnings.filter(m => isInline(m)),
+    messages: results.messages.filter(m => isInline(m)),
+    markdowns: results.markdowns.filter(m => isInline(m)),
+  })
+}
+
+export function regularResults(results: DangerResults): DangerResults {
+  return sortResults({
+    fails: results.fails.filter(m => !isInline(m)),
+    warnings: results.warnings.filter(m => !isInline(m)),
+    messages: results.messages.filter(m => !isInline(m)),
+    markdowns: results.markdowns.filter(m => !isInline(m)),
+  })
+}
+
+export function mergeResults(results1: DangerResults, results2: DangerResults): DangerResults {
+  return sortResults({
+    fails: results1.fails.concat(results2.fails),
+    warnings: results1.warnings.concat(results2.warnings),
+    messages: results1.messages.concat(results2.messages),
+    markdowns: results1.markdowns.concat(results2.markdowns),
+  })
+}
+
+export function sortResults(results: DangerResults): DangerResults {
+  let sortByFile = (a: Violation, b: Violation): number => {
+    if (a.file === undefined) {
+      return -1
+    }
+    if (b.file === undefined) {
+      return 1
+    }
+
+    if (a.file == b.file) {
+      if (a.line == undefined) {
+        return -1
+      }
+      if (b.line == undefined) {
+        return 1
+      }
+
+      if (a.line < b.line) {
+        return -1
+      } else if (a.line > b.line) {
+        return 1
+      } else {
+        return 0
+      }
+    }
+
+    if (a.file < b.file) {
+      return -1
+    } else {
+      return 1
+    }
+  }
+
+  return {
+    fails: results.fails.sort(sortByFile),
+    warnings: results.warnings.sort(sortByFile),
+    messages: results.messages.sort(sortByFile),
+    markdowns: results.markdowns.sort(sortByFile),
+  }
+}
+
+export async function resultsIntoInlineResults(results: DangerResults): Promise<DangerInlineResults[]> {
+  let dangerInlineResults: DangerInlineResults[] = []
+
+  let violationsIntoInlineResults = (kind: string) => {
+    for (let violation of results[kind]) {
+      if (violation.file && violation.line) {
+        let findInlineResult = dangerInlineResults.find(r => r.file == violation.file && r.line == violation.line)
+        let inlineResult = findInlineResult
+          ? findInlineResult
+          : { file: violation.file, line: violation.line, results: emptyDangerResults }
+        inlineResult.results[kind].push(violation)
+        dangerInlineResults.push(inlineResult)
+      }
+    }
+  }
+  ;["fails", "warnings", "messages", "markdowns"].forEach(violationsIntoInlineResults)
+
+  return new Promise<DangerInlineResults[]>(resolve => resolve(dangerInlineResults))
+}

source/dsl/Violation.ts

@@ -9,7 +9,17 @@ export interface Violation {
    * @type {string}
    */
   message: string
+
+  /**
+   * Optional path to the file
+   * @type {string}
+   */
   file?: string
+
+  /**
+   * Optional line in the file
+   * @type {string}
+   */
   line?: number
 }
 

source/runner/Dangerfile.ts

@@ -54,7 +54,7 @@ export interface DangerContext {
    *
    * @param {MarkdownString} message the String to output
    */
-  markdown(message: MarkdownString): void
+  markdown(message: MarkdownString, file?: string, line?: number): void
 
   /**
    * The root Danger object. This contains all of the metadata you
@@ -97,7 +97,8 @@ export function contextForDanger(dsl: DangerDSLType): DangerContext {
   const warn = (message: MarkdownString, file?: string, line?: number) => results.warnings.push({ message, file, line })
   const message = (message: MarkdownString, file?: string, line?: number) =>
     results.messages.push({ message, file, line })
-  const markdown = (message: MarkdownString) => results.markdowns.push(message)
+  const markdown = (message: MarkdownString, file?: string, line?: number) =>
+    results.markdowns.push({ message, file, line })
 
   return {
     schedule,

source/runner/Executor.ts

@@ -2,7 +2,15 @@ import { contextForDanger, DangerContext } from "./Dangerfile"
 import { DangerDSL, DangerDSLType } from "../dsl/DangerDSL"
 import { CISource } from "../ci_source/ci_source"
 import { Platform } from "../platforms/platform"
-import { DangerResults } from "../dsl/DangerResults"
+import {
+  inlineResults,
+  regularResults,
+  mergeResults,
+  DangerResults,
+  DangerInlineResults,
+  resultsIntoInlineResults,
+  emptyDangerResults,
+} from "../dsl/DangerResults"
 import { template as githubResultsTemplate } from "./templates/githubIssueTemplate"
 import exceptionRaisedTemplate from "./templates/exceptionRaisedTemplate"
 
@@ -12,7 +20,8 @@ import { sentence, href } from "./DangerUtils"
 import { DangerRunner } from "./runners/runner"
 import { jsonToDSL } from "./jsonToDSL"
 import { jsonDSLGenerator } from "./dslGenerator"
-import { Violation, isInline as isInlineViolation } from "../dsl/Violation"
+import { Violation } from "../dsl/Violation"
+import { GitDSL } from "../dsl/GitDSL"
 
 // This is still badly named, maybe it really should just be runner?
 
@@ -100,9 +109,9 @@ export class Executor {
   async handleResults(results: DangerResults, danger: DangerDSLType) {
     this.d(`Got Results back, current settings`, this.options)
     if (this.options.stdoutOnly || this.options.jsonOnly) {
-      this.handleResultsPostingToSTDOUT(results)
+      await this.handleResultsPostingToSTDOUT(results)
     } else {
-      this.handleResultsPostingToPlatform(results, danger)
+      await this.handleResultsPostingToPlatform(results, danger)
     }
   }
   /**
@@ -198,37 +207,12 @@ export class Executor {
         console.log("Found only messages, passing those to review.")
       }
 
-      let inlineResults: DangerResults = {
-        warnings: results.warnings.filter(m => isInlineViolation(m)),
-        fails: results.fails.filter(m => isInlineViolation(m)),
-        messages: results.messages.filter(m => isInlineViolation(m)),
-        markdowns: [],
-      }
-      // TODO: Get current inline comments, if any of the old ones is not present
-      // in the new ones - delete.
-      let sendViolation = (violation: Violation, kind: string): void => {
-        let file = violation.file
-        let line = violation.line
-        if (file && line) {
-          let commit = danger.github.pr.head
-          console.log("Creating comment. Commit: " + commit.sha + ', file: "' + file + '", line: ' + line)
-          this.platform.createInlineComment(danger.git, kind + ": " + violation.message, file, line)
-        }
-      }
-      // TODO: Check if any of the inline comments were not accepted
-      // add them to regular comments instead.
-      inlineResults.warnings.forEach(v => sendViolation(v, "warnings"))
-      inlineResults.fails.forEach(v => sendViolation(v, "fails"))
-      inlineResults.messages.forEach(v => sendViolation(v, "messages"))
-
-      let regularResults: DangerResults = {
-        warnings: results.warnings.filter(m => !isInlineViolation(m)),
-        fails: results.fails.filter(m => !isInlineViolation(m)),
-        messages: results.messages.filter(m => !isInlineViolation(m)),
-        markdowns: results.markdowns,
-      }
+      let inline = inlineResults(results)
+      let inlineLeftovers = await this.sendInlineComments(inline, danger.git)
+      let regular = regularResults(results)
+      let mergedResults = mergeResults(regular, inlineLeftovers)
 
-      const comment = githubResultsTemplate(dangerID, regularResults)
+      const comment = githubResultsTemplate(dangerID, mergedResults)
       await this.platform.updateOrCreateComment(dangerID, comment)
     }
 
@@ -238,6 +222,36 @@ export class Executor {
     }
   }
 
+  /**
+   * Send inline comments
+   * Returns these violations that didn't pass the validation (e.g. incorrect file/line)
+   *
+   * @param results Results with inline violations
+   */
+  sendInlineComments(results: DangerResults, git: GitDSL): Promise<DangerResults> {
+    // TODO: Get current inline comments, if any of the old ones is not present
+    // in the new ones - delete.
+    return resultsIntoInlineResults(results).then(inlineResults => {
+      let promises: Promise<DangerResults>[] = inlineResults.map(inlineResult => {
+        return this.sendInlineComment(git, inlineResult)
+          .then(_ => emptyDangerResults)
+          .catch(_ => {
+            return inlineResult.results
+          })
+      })
+      return Promise.all(promises).then(results => {
+        return new Promise<DangerResults>(resolve => {
+          resolve(results.reduce((acc, r) => mergeResults(acc, r), emptyDangerResults))
+        })
+      })
+    })
+  }
+
+  async sendInlineComment(git: GitDSL, inlineResults: DangerInlineResults): Promise<any> {
+    let template = githubResultsTemplate(this.options.dangerID, inlineResults.results)
+    return await this.platform.createInlineComment(git, template, inlineResults.file, inlineResults.line)
+  }
+
   /**
    * Takes an error (maybe a bad eval) and provides a DangerResults compatible object
    * @param error Any JS error
@@ -250,7 +264,7 @@ export class Executor {
       fails: [{ message: "Running your Dangerfile has Failed" }],
       warnings: [],
       messages: [],
-      markdowns: [exceptionRaisedTemplate(error)],
+      markdowns: [{ message: exceptionRaisedTemplate(error) }],
     }
   }
 }

source/runner/_tests/fixtures/ExampleDangerResults.ts

@@ -39,7 +39,7 @@ export const summaryResults: DangerResults = {
   fails: [{ message: "Failing message Failing message" }],
   warnings: [{ message: "Warning message Warning message" }],
   messages: [{ message: "message" }],
-  markdowns: ["markdown"],
+  markdowns: [{ message: "markdown" }],
 }
 
 export const asyncResults: DangerResults = {

source/runner/runners/_tests/vm2.test.ts

@@ -92,7 +92,7 @@ runners.forEach(run => {
 
         expect(results).toEqual({
           fails: [{ message: "this is a failure" }],
-          markdowns: ["this is a *markdown*"],
+          markdowns: [{ message: "this is a *markdown*" }],
           messages: [{ message: "this is a message" }],
           warnings: [{ message: "this is a warning" }],
         })
@@ -108,7 +108,7 @@ runners.forEach(run => {
         )
 
         expect(results.fails[0].message).toContain("Danger failed to run")
-        expect(results.markdowns[0]).toContain("hello is not defined")
+        expect(results.markdowns[0].message).toContain("hello is not defined")
       })
 
       it("handles relative imports correctly in Babel", async () => {
@@ -148,7 +148,7 @@ runners.forEach(run => {
         expect(results).toEqual({
           fails: [{ message: "Asynchronous Failure" }],
           messages: [{ message: "Asynchronous Message" }],
-          markdowns: ["Asynchronous Markdown"],
+          markdowns: [{ message: "Asynchronous Markdown" }],
           warnings: [{ message: "Asynchronous Warning" }],
         })
       })
@@ -261,7 +261,7 @@ runners.forEach(run => {
         )
 
         expect(results.fails[0].message).toContain("Danger failed to run")
-        expect(results.markdowns[0]).toContain("Error: failure")
+        expect(results.markdowns[0].message).toContain("Error: failure")
       })
     })
   })

source/runner/runners/utils/resultsForCaughtError.ts

@@ -22,7 +22,7 @@ ${error.stack}
 ${code}
 \`\`\`
   `
-  return { fails: [{ message: failure }], warnings: [], markdowns: [errorMD], messages: [] }
+  return { fails: [{ message: failure }], warnings: [], markdowns: [{ message: errorMD }], messages: [] }
 }
 
 export default resultsForCaughtError

source/runner/templates/_tests/_githubIssueTemplates.test.ts

@@ -58,7 +58,7 @@ describe("generating messages", () => {
       fails: [{ message: "**Failure:** Something failed!" }],
       warnings: [{ message: "_Maybe you meant to run `yarn install`?_" }],
       messages: [{ message: "```ts\nfunction add(a: number, b: number): number {\n  return a + b\n}\n```" }],
-      markdowns: ["List of things:\n\n* one\n* two\n* three\n"],
+      markdowns: [{ message: "List of things:\n\n* one\n* two\n* three\n" }],
     })
 
     expect(issues).toMatchSnapshot()

source/runner/templates/githubIssueTemplate.ts

@@ -79,7 +79,7 @@ ${buildSummaryMessage(dangerID, results)}
 ${table("Fails", "no_entry_sign", results.fails)}
 ${table("Warnings", "warning", results.warnings)}
 ${table("Messages", "book", results.messages)}
-${results.markdowns.join("\n\n")}
+${results.markdowns.map(v => v.message).join("\n\n")}
 <p align="right">
   ${dangerSignaturePostfix}
 </p>