Improve Golang annotation positioning, and improve project structure

src/annotation-result.ts

@@ -1,3 +1,4 @@
+import { AnnotationSource } from './annotation-source'
 import { Dependency } from './dependency'
 
 export class AnnotationResult {
@@ -29,3 +30,22 @@ export class AnnotationResult {
     this.endLine = endLine
   }
 }
+
+export function createAnnotation(
+  annotationSource: AnnotationSource,
+  annotationLevel: 'notice' | 'warning' | 'failure',
+  title: string,
+  message: string,
+): AnnotationResult {
+  const { dependency, filename, line } = annotationSource
+
+  return new AnnotationResult(
+    title,
+    message,
+    annotationLevel,
+    dependency,
+    filename,
+    line,
+    line,
+  )
+}

src/annotation-source.ts

@@ -0,0 +1,7 @@
+import { Dependency } from './dependency'
+
+export interface AnnotationSource {
+  dependency: Dependency
+  filename: string
+  line: number
+}

src/dependency-check-gopkg.ts

@@ -1,10 +1,10 @@
 import { Context } from 'probot' // eslint-disable-line no-unused-vars
 import toml from 'toml'
+
 import { AnalysisResult } from './analysis-result'
+import { createAnnotation } from './annotation-result'
+import { AnnotationSource } from './annotation-source'
 import { Dependency } from './dependency'
-import { AnnotationSource,
-         createAnnotation,
-         findLineInFileContent } from './dependency-check'
 
 export async function checkGopkgFileAsync(
   analysisResult: AnalysisResult,
@@ -22,45 +22,94 @@ export async function checkGopkgFileAsync(
   context.log.debug(`get contents response for ${gopkgLockFilename}: ${gopkgLockContentsResponse.status}`)
 
   const gopkgTomlContents = Buffer.from(gopkgTomlContentsResponse.data.content, 'base64').toString('utf8')
-  const gopkgTomlContentsToml = toml.parse(gopkgTomlContents)
   const gopkgLockContents = Buffer.from(gopkgLockContentsResponse.data.content, 'base64').toString('utf8')
   const gopkgLockContentsToml = toml.parse(gopkgLockContents)
 
   await checkGoDependenciesAsync(
     gopkgTomlContents, gopkgLockContents,
-    getDependenciesFromGopkg(gopkgTomlContentsToml, gopkgLockContentsToml),
+    getDependenciesFromGopkg(gopkgLockContentsToml),
     gopkgTomlFilename, gopkgLockFilename,
     analysisResult)
 }
 
-function getDependenciesFromGopkg(gopkgTomlContentsToml: any, gopkgLockContentsToml: any): Dependency[] {
+interface GopkgLockProject {
+  digest: string,
+  name: string,
+  source?: string,
+  packages: string[],
+  pruneopts?: string,
+
+  revision: string,
+  branch?: string,
+  version?: string,
+}
+
+function getDependenciesFromGopkg(gopkgLockContentsToml: any): Dependency[] {
   const dependencies: Dependency[] = []
 
-  for (const tomlDep of gopkgLockContentsToml.projects) {
+  for (const tomlDep of gopkgLockContentsToml.projects as GopkgLockProject[]) {
+    const rawRefType = getRawRefType(tomlDep)
     dependencies.push({
       name: tomlDep.name,
       url: tomlDep.source ? tomlDep.source : tomlDep.name,
 
-      refType: getRefType(gopkgTomlContentsToml, tomlDep),
+      rawRefType,
+      refName: rawRefType ? (tomlDep as any)[rawRefType] : undefined,
+      refType: getRefType(tomlDep),
     })
   }
 
   return dependencies
 }
 
-function getRefType(gopkgTomlContentsToml: any, tomlDep: any): 'commit' | 'tag' | 'branch' | 'unknown' {
+function getRawRefType(tomlDep: GopkgLockProject): string | undefined {
   if (tomlDep.version) {
-    return 'tag'
+    return 'version'
   } else if (tomlDep.branch) {
     return 'branch'
-  } else {
+  } else if (tomlDep.revision) {
-    const override: any = gopkgTomlContentsToml.override.find((o: any) => o.name === tomlDep.name)
+    return 'revision'
-    if (override && override.revision) {
+  }
+
+  return undefined
+}
+
+function getRefType(tomlDep: GopkgLockProject): 'commit' | 'tag' | 'branch' | 'unknown' {
+  switch (getRawRefType(tomlDep)) {
+    case 'version':
+      return 'tag'
+    case 'branch':
+      return 'branch'
+    case 'revision':
       return 'commit'
+    default:
+      return 'unknown'
   }
 }
 
-  return 'unknown'
+interface SearchArgs {
+  projectName: string,
+  projectLineSubstring: string
+}
+
+export function findLineInTomlFileContent(contents: string, searchArgs: SearchArgs): number {
+  const projectNameIndex = contents.indexOf(searchArgs.projectName)
+  if (projectNameIndex < 0) {
+    return -1
+  }
+
+  const projectStartIndex = contents.lastIndexOf('[[', projectNameIndex)
+  if (projectStartIndex < 0) {
+    return projectNameIndex
+  }
+  const index = contents.indexOf(searchArgs.projectLineSubstring, projectStartIndex)
+  if (index < 0) {
+    return projectNameIndex
+  }
+
+  const line = contents.substr(0, index).split('\n').length
+
+  return line
 }
 
 export async function checkGoDependenciesAsync(
@@ -77,40 +126,49 @@ export async function checkGoDependenciesAsync(
   result.checkedDependencyCount += dependencies.length
 
   for (const dependency of dependencies) {
-    const url = dependency.url
+    const name = dependency.name
-    let line = findLineInFileContent(gopkgTomlContents, `name = "${url}"`)
-    let filename = gopkgTomlFilename
-    if (line < 0) {
-      line = findLineInFileContent(gopkgLockContents, `name = "${url}"`)
-      filename = gopkgLockFilename
-    }
     const refType = dependency.refType
     if (!refType) {
       continue
     }
 
+    const searchArgs: SearchArgs = {
+      projectLineSubstring: `${dependency.rawRefType} = "${dependency.refName}"`,
+      projectName: `name = "${name}"`,
+    }
+    let line = findLineInTomlFileContent(gopkgTomlContents, searchArgs)
+    let filename = gopkgTomlFilename
+    if (line < 0) {
+      line = findLineInTomlFileContent(gopkgLockContents, searchArgs)
+      filename = gopkgLockFilename
+    }
+
     const annotation: AnnotationSource = {
       dependency,
       filename,
       line,
     }
-    const newAnnotation = (level: 'notice' | 'warning' | 'failure', title: string, message: string) => {
+    const newAnnotation = (level: 'notice' | 'warning' | 'failure', message: string) => {
+      const title = `Dependency '${name}' is locked with ${dependency.rawRefType} '${dependency.refName}'.`
       result.annotations.push(createAnnotation(annotation, level, title, message))
     }
     switch (refType) {
       case 'tag':
         continue
       case 'commit':
-        newAnnotation('notice', `Dependency '${url}' is not locked with a tag/release.`,
+        newAnnotation('notice',
                       `A commit SHA is not a deterministic dependency locator.
-If the commit is overwritten by a force-push, it will be impossible to rebuild the same output in the future.`,
+If the commit is overwritten by a force-push, it will be impossible to rebuild the same output in the future.
+
+Please lock the dependency with a tag/release.`,
         )
         break
       case 'branch':
         newAnnotation('notice', // TODO: change this to 'failure' once we've fixed issues in the codebase
-                      `Dependency '${url}' is not locked with a tag/release.`,
                       `A branch is not a deterministic dependency locator.
-If the branch advances, it will be impossible to rebuild the same output in the future.`,
+If the branch advances, it will be impossible to rebuild the same output in the future.
+
+Please lock the dependency with a tag/release.`,
         )
         break
     }

src/dependency-check-json.ts

@@ -1,10 +1,11 @@
 import { Context } from 'probot' // eslint-disable-line no-unused-vars
+
 import { AnalysisResult } from './analysis-result'
+import { createAnnotation } from './annotation-result'
+import { AnnotationSource } from './annotation-source'
 import { Dependency } from './dependency'
-import { AnnotationSource,
+import { findLineInFileContent,
-         createAnnotation,
+         slowGetRefTypeAsync } from './utils'
-         findLineInFileContent,
-         slowGetRefTypeAsync } from './dependency-check'
 
 export async function checkPackageFileAsync(
   analysisResult: AnalysisResult,

src/dependency.ts

@@ -1,5 +1,7 @@
 export interface Dependency {
   name: string
   url: string
+  rawRefType?: string,
   refType?: 'commit' | 'tag' | 'branch' | 'unknown'
+  refName?: string
 }

src/index.ts

@@ -3,6 +3,7 @@
 import Octokit from '@octokit/rest'
 import Humanize from 'humanize-plus'
 import { Application, Context } from 'probot' // eslint-disable-line no-unused-vars
+
 import { AnalysisResult } from './analysis-result'
 import { AnnotationResult } from './annotation-result'
 import { checkGopkgFileAsync } from './dependency-check-gopkg'

src/utils.ts

@@ -1,13 +1,5 @@
 import Octokit from '@octokit/rest'
 import { Context } from 'probot'
-import { AnnotationResult } from './annotation-result'
-import { Dependency } from './dependency'
-
-export interface AnnotationSource {
-  dependency: Dependency
-  filename: string
-  line: number
-}
 
 export function findLineInFileContent(contents: string, substring: string): number {
   const index = contents.indexOf(substring)
@@ -15,39 +7,20 @@ export function findLineInFileContent(contents: string, substring: string): numb
     return -1
   }
 
-  const lines = contents.split('\n')
   const line = contents.substr(0, index).split('\n').length
 
-  const startOfLineIndex = (() => {
+  // const lines = contents.split('\n')
-    const x = lines.slice(0)
+  // const startOfLineIndex = (() => {
-    x.splice(line - 1)
+  //   const x = lines.slice(0)
-    return x.join('\n').length + (x.length > 0 ? 1 : 0)
+  //   x.splice(line - 1)
-  })()
+  //   return x.join('\n').length + (x.length > 0 ? 1 : 0)
+  // })()
 
-  const col = index - startOfLineIndex
+  // const col = index - startOfLineIndex
 
   return line
 }
 
-export function createAnnotation(
-  annotationSource: AnnotationSource,
-  annotationLevel: 'notice' | 'warning' | 'failure',
-  title: string,
-  message: string,
-): AnnotationResult {
-  const { dependency, filename, line } = annotationSource
-
-  return new AnnotationResult(
-    title,
-    message,
-    annotationLevel,
-    dependency,
-    filename,
-    line,
-    line,
-  )
-}
-
 export async function slowGetRefTypeAsync(
   context: Context,
   address: string,