From ce15a110dec3ba6dff2de641182774981fd67645 Mon Sep 17 00:00:00 2001 From: Pedro Pombeiro Date: Thu, 24 Jan 2019 11:48:51 +0100 Subject: [PATCH] Improve Golang annotation positioning, and improve project structure --- src/annotation-result.ts | 20 +++++ src/annotation-source.ts | 7 ++ src/dependency-check-gopkg.ts | 114 +++++++++++++++++++------- src/dependency-check-json.ts | 9 +- src/dependency.ts | 2 + src/index.ts | 1 + src/{dependency-check.ts => utils.ts} | 41 ++------- 7 files changed, 128 insertions(+), 66 deletions(-) create mode 100644 src/annotation-source.ts rename src/{dependency-check.ts => utils.ts} (65%) diff --git a/src/annotation-result.ts b/src/annotation-result.ts index 6bbcd61..9a48121 100644 --- a/src/annotation-result.ts +++ b/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, + ) +} diff --git a/src/annotation-source.ts b/src/annotation-source.ts new file mode 100644 index 0000000..61ebc16 --- /dev/null +++ b/src/annotation-source.ts @@ -0,0 +1,7 @@ +import { Dependency } from './dependency' + +export interface AnnotationSource { + dependency: Dependency + filename: string + line: number +} diff --git a/src/dependency-check-gopkg.ts b/src/dependency-check-gopkg.ts index b973c5c..841efe4 100644 --- a/src/dependency-check-gopkg.ts +++ b/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 { - const override: any = gopkgTomlContentsToml.override.find((o: any) => o.name === tomlDep.name) - if (override && override.revision) { - return 'commit' - } + } else if (tomlDep.revision) { + return 'revision' } - return 'unknown' + 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' + } +} + +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 - let line = findLineInFileContent(gopkgTomlContents, `name = "${url}"`) - let filename = gopkgTomlFilename - if (line < 0) { - line = findLineInFileContent(gopkgLockContents, `name = "${url}"`) - filename = gopkgLockFilename - } + const name = dependency.name 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 } diff --git a/src/dependency-check-json.ts b/src/dependency-check-json.ts index f27cb70..2287e3e 100644 --- a/src/dependency-check-json.ts +++ b/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, - createAnnotation, - findLineInFileContent, - slowGetRefTypeAsync } from './dependency-check' +import { findLineInFileContent, + slowGetRefTypeAsync } from './utils' export async function checkPackageFileAsync( analysisResult: AnalysisResult, diff --git a/src/dependency.ts b/src/dependency.ts index 4657f06..d2dd702 100644 --- a/src/dependency.ts +++ b/src/dependency.ts @@ -1,5 +1,7 @@ export interface Dependency { name: string url: string + rawRefType?: string, refType?: 'commit' | 'tag' | 'branch' | 'unknown' + refName?: string } diff --git a/src/index.ts b/src/index.ts index a479046..ddc6fd5 100644 --- a/src/index.ts +++ b/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' diff --git a/src/dependency-check.ts b/src/utils.ts similarity index 65% rename from src/dependency-check.ts rename to src/utils.ts index 150ab6e..d2c4197 100644 --- a/src/dependency-check.ts +++ b/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 x = lines.slice(0) - x.splice(line - 1) - return x.join('\n').length + (x.length > 0 ? 1 : 0) - })() + // const lines = contents.split('\n') + // const startOfLineIndex = (() => { + // const x = lines.slice(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,