From 95057f39f5fc5e404017ffb4d8f4b6efdf245600 Mon Sep 17 00:00:00 2001 From: Tobias Date: Sun, 17 Mar 2024 18:14:08 +0100 Subject: [PATCH] Refactor `check-header` script Refactor check header script to only consider the `endYear` in copyrigh headers. Previously we also had a mechanism in place to validate the start year of a copyright range. However, this approach was flawed because - It could not consider files that predate the initial inception of the repository - had a hard time with determining the correct start year for files that have been renamed/moved So in the end we opted for ignoring startYear validations anyways and only considered endYear errors. Therefore this PR removes the entire `startYear` validation and we only check for single year or end year violations. This makes the whole check process faster and less error prone. In addition, there was an issue with files that had multiple copyright years defined (e.g. `contribution-provider` in glsp-client). This is now fixed. In addition, align version numbers with the other GLSP projects so that the dev-packages can be included in the Release train. Also: Minor fix for `release-vscode-integration` --- dev-packages/cli/package.json | 4 +- dev-packages/cli/src/commands/check-header.ts | 166 +++++------------- .../release/release-vscode-integration.ts | 3 +- dev-packages/cli/src/util/git-util.ts | 54 ------ dev-packages/config-test/package.json | 6 +- dev-packages/config/package.json | 8 +- dev-packages/dev/package.json | 8 +- dev-packages/eslint-config/package.json | 2 +- dev-packages/mocha-config/package.json | 2 +- dev-packages/nyc-config/package.json | 2 +- dev-packages/prettier-config/package.json | 2 +- dev-packages/ts-config/package.json | 2 +- lerna.json | 2 +- package.json | 2 +- 14 files changed, 62 insertions(+), 201 deletions(-) diff --git a/dev-packages/cli/package.json b/dev-packages/cli/package.json index 004cc72..4e5559a 100644 --- a/dev-packages/cli/package.json +++ b/dev-packages/cli/package.json @@ -1,6 +1,6 @@ { "name": "@eclipse-glsp/cli", - "version": "2.0.0", + "version": "2.2.0-next", "description": "CLI Tooling & scripts for GLSP components", "keywords": [ "eclipse", @@ -49,7 +49,7 @@ "shelljs": "^0.8.5" }, "devDependencies": { - "@eclipse-glsp/config": "~2.0.0", + "@eclipse-glsp/config": "2.2.0-next", "@types/glob": "^8.1.0", "@types/node-fetch": "^2.6.6", "@types/readline-sync": "^1.4.5", diff --git a/dev-packages/cli/src/commands/check-header.ts b/dev-packages/cli/src/commands/check-header.ts index 75c8aad..52b3d95 100644 --- a/dev-packages/cli/src/commands/check-header.ts +++ b/dev-packages/cli/src/commands/check-header.ts @@ -21,14 +21,7 @@ import * as minimatch from 'minimatch'; import * as readline from 'readline-sync'; import * as sh from 'shelljs'; import { baseCommand, configureShell, getShellConfig } from '../util/command-util'; -import { - getChangesOfLastCommit, - getFirstCommit, - getFirstModificationDate, - getInitialCommit, - getLastModificationDate, - getUncommittedChanges -} from '../util/git-util'; +import { getChangesOfLastCommit, getLastModificationDate, getUncommittedChanges } from '../util/git-util'; import { LOGGER } from '../util/logger'; import { validateGitDirectory } from '../util/validation-util'; @@ -40,24 +33,19 @@ export interface HeaderCheckOptions { json: boolean; excludeDefaults: boolean; autoFix: boolean; - severity: Severity; } const checkTypes = ['full', 'changes', 'lastCommit'] as const; -type CheckType = typeof checkTypes[number]; - -const severityTypes = ['error', 'warn', 'ok'] as const; - -type Severity = typeof severityTypes[number]; +type CheckType = (typeof checkTypes)[number]; const DEFAULT_EXCLUDES = ['**/@(node_modules|lib|dist|bundle)/**']; -const YEAR_RANGE_REGEX = /\d{4}(?:-d{4})?/g; -const HEADER_PATTERN = 'Copyright \\([cC]\\) \\d{4}(-d{4})?'; +const YEAR_RANGE_REGEX = /\d{4}/g; +const HEADER_PATTERN = 'Copyright \\([cC]\\) \\d{4}'; const AUTO_FIX_MESSAGE = 'Fix copyright header violations'; export const CheckHeaderCommand = baseCommand() // .name('checkHeaders') - .description('Validates the copyright year range of license header files') + .description('Validates the copyright year range (end year) of license header files') .argument('', 'The starting directory for the check', validateGitDirectory) .addOption( new Option( @@ -80,11 +68,6 @@ export const CheckHeaderCommand = baseCommand() // 'Disables the default excludes patterns. Only explicitly passed exclude patterns (-e, --exclude) are considered' ) .option('-j, --json', 'Also persist validation results as json file', false) - .addOption( - new Option('-s, --severity ', 'The severity of validation results that should be printed.') - .choices(severityTypes) - .default('error', '"error" (only)') - ) .option('-a, --autoFix', 'Auto apply & commit fixes without prompting the user', false) .action(checkHeaders); @@ -127,17 +110,11 @@ function getFiles(rootDir: string, options: HeaderCheckOptions): string[] { } function validate(rootDir: string, files: string[], options: HeaderCheckOptions): ValidationResult[] { - // Derives all files with valid headers, their copyright years and all files with no or invalid headers + // Derives all files with valid headers and all files with no or invalid headers const filesWithHeader = sh.grep('-l', HEADER_PATTERN, files).stdout.trim().split('\n'); - const copyrightYears = sh - .grep(HEADER_PATTERN, files) - .stdout.trim() - .split('\n') - .map(line => line.match(YEAR_RANGE_REGEX)!.map(string => Number.parseInt(string, 10))); const noHeaders = files.filter(file => !filesWithHeader.includes(file)); const results: ValidationResult[] = []; - const allFilesLength = files.length; // Create validation results for all files with no or invalid headers @@ -147,7 +124,7 @@ function validate(rootDir: string, files: string[], options: HeaderCheckOptions) } noHeaders.forEach((file, i) => { printFileProgress(i + 1, allFilesLength, `Validating ${file}`); - results.push({ file: path.resolve(rootDir, file), violation: 'noOrMissingHeader', severity: 'error' }); + results.push({ file: path.resolve(rootDir, file), violation: 'noOrMissingHeader' }); }); // Performance optimization: avoid retrieving the dates for each individual file by precalculating the endYear if possible. @@ -161,22 +138,20 @@ function validate(rootDir: string, files: string[], options: HeaderCheckOptions) // Create validation results for all files with valid headers filesWithHeader.forEach((file, i) => { printFileProgress(i + 1 + noHeadersLength, allFilesLength, `Validating ${file}`); - + const copyrightLine = sh.head({ '-n': 2 }, file).stdout.trim().split('\n')[1]; + const copyRightYears = copyrightLine.match(YEAR_RANGE_REGEX)!; + const currentStartYear = Number.parseInt(copyRightYears[0], 10); + const currentEndYear = copyRightYears[1] ? Number.parseInt(copyRightYears[1], 10) : undefined; const result: DateValidationResult = { - currentStartYear: copyrightYears[i].shift()!, - expectedStartYear: getFirstModificationDate(file, rootDir, AUTO_FIX_MESSAGE)!.getFullYear(), - currentEndYear: copyrightYears[i].shift(), + currentStartYear, + currentEndYear, expectedEndYear: defaultEndYear ?? getLastModificationDate(file, rootDir, AUTO_FIX_MESSAGE)!.getFullYear(), file, - severity: 'ok', violation: 'none' }; - if (result.expectedStartYear === result.expectedEndYear) { - validateSingleYear(result); - } else { - validateTimePeriod(result); - } + validateEndYear(result); + results.push(result); }); @@ -186,57 +161,16 @@ function validate(rootDir: string, files: string[], options: HeaderCheckOptions) return results; } -function validateSingleYear(result: DateValidationResult): void { - const { currentStartYear, expectedStartYear, currentEndYear } = result; - result.violation = 'invalidCopyrightYear'; - result.severity = 'error'; - - if (!currentEndYear) { - if (currentStartYear === expectedStartYear) { - result.violation = 'none'; - result.severity = 'ok'; - } - return; - } - - // Cornercase: For files of the initial contribution the copyright header predates the first git modification date. - // => declare as warning if not part of the initial contribution. - if (expectedStartYear === currentEndYear && currentStartYear < expectedStartYear) { - if (getFirstCommit(result.file) === getInitialCommit()) { - result.violation = 'none'; - result.severity = 'ok'; - } else { - result.severity = 'warn'; - } - } -} - -function validateTimePeriod(result: DateValidationResult): void { - const { currentStartYear, expectedStartYear, expectedEndYear, currentEndYear } = result; +function validateEndYear(result: DateValidationResult): void { + const { currentStartYear, expectedEndYear, currentEndYear } = result; + result.violation = 'invalidEndYear'; - result.violation = 'incorrectCopyrightPeriod'; - result.severity = 'error'; - if (!currentEndYear) { - result.severity = 'error'; - return; - } + const valid = currentEndYear ? currentEndYear === expectedEndYear : currentStartYear === expectedEndYear; - if (currentStartYear === expectedStartYear && currentEndYear === expectedEndYear) { + if (valid) { result.violation = 'none'; - result.severity = 'ok'; return; } - - // Cornercase: For files of the initial contribution the copyright header predates the first git modification date. - // => declare as warning if not part of the initial contribution. - if (currentEndYear === expectedEndYear && currentStartYear < expectedEndYear) { - if (getFirstCommit(result.file) === getInitialCommit()) { - result.violation = 'none'; - result.severity = 'ok'; - } else { - result.severity = 'warn'; - } - } } function printFileProgress(currentFileCount: number, maxFileCount: number, message: string, clear = true): void { @@ -253,14 +187,9 @@ function printFileProgress(currentFileCount: number, maxFileCount: number, messa export function handleValidationResults(rootDir: string, results: ValidationResult[], options: HeaderCheckOptions): void { LOGGER.newLine(); LOGGER.info(`Header validation for ${results.length} files completed`); - const violations = results.filter(result => result.severity === 'error'); + const violations = results.filter(result => result.violation !== 'none'); // Adjust results to print based on configured severity level - let toPrint = results; - if (options.severity === 'error') { - toPrint = violations; - } else if (options.severity === 'warn') { - toPrint = results.filter(result => result.severity !== 'ok'); - } + const toPrint = violations; LOGGER.info(`Found ${toPrint.length} copyright header violations:`); LOGGER.newLine(); @@ -274,9 +203,7 @@ export function handleValidationResults(rootDir: string, results: ValidationResu } if (violations.length > 0 && (options.autoFix || readline.keyInYN('Do you want automatically fix copyright year range violations?'))) { - const toFix = violations.filter( - violation => violation.severity === 'error' && isDateValidationResult(violation) - ) as DateValidationResult[]; + const toFix = violations.filter(violation => isDateValidationResult(violation)) as DateValidationResult[]; fixViolations(rootDir, toFix, options); } @@ -284,43 +211,32 @@ export function handleValidationResults(rootDir: string, results: ValidationResu } function toPrintMessage(result: ValidationResult): string { - const colors: Record = { - error: '\x1b[31m', - warn: '\x1b[33m', - ok: '\x1b[32m' - } as const; - - if ( - isDateValidationResult(result) && - (result.violation === 'incorrectCopyrightPeriod' || result.violation === 'invalidCopyrightYear') - ) { - const expected = - result.expectedStartYear !== result.expectedEndYear - ? `${result.expectedStartYear}-${result.expectedEndYear}` - : result.expectedStartYear.toString(); - const actual = result.currentEndYear ? `${result.currentStartYear}-${result.currentEndYear}` : result.currentStartYear.toString(); - const message = result.violation === 'incorrectCopyrightPeriod' ? 'Invalid copyright period' : 'Invalid copyright year'; - return `${colors[result.severity]} ${message}! Expected '${expected}' but is '${actual}'`; + const error = '\x1b[31m'; + const info = '\x1b[32m'; + + if (isDateValidationResult(result) && result.violation === 'invalidEndYear') { + const expected = result.expectedEndYear.toString(); + const actual = result.currentEndYear + ? `${result.currentEndYear} (${result.currentStartYear}-${result.currentEndYear})` + : result.currentStartYear.toString(); + const message = 'Invalid copyright end year'; + return `${error} ${message}! Expected end year '${expected}' but is '${actual}'`; } else if (result.violation === 'noOrMissingHeader') { - return `${colors[result.severity]} No or invalid copyright header!`; + return `${error} No or invalid copyright header!`; } - return `${colors[result.severity]} OK`; + return `${info} OK`; } function fixViolations(rootDir: string, violations: DateValidationResult[], options: HeaderCheckOptions): void { LOGGER.newLine(); violations.forEach((violation, i) => { printFileProgress(i + 1, violations.length, `Fix ${violation.file}`, false); - const fixedStartYear = - violation.currentStartYear < violation.expectedStartYear ? violation.currentStartYear : violation.expectedStartYear; const currentRange = `${violation.currentStartYear}${violation.currentEndYear ? '-' + violation.currentEndYear : ''}`; - - let fixedRange = `${fixedStartYear}`; - if (violation.expectedEndYear !== violation.expectedStartYear || fixedStartYear !== violation.expectedStartYear) { - fixedRange = `${fixedStartYear}-${violation.expectedEndYear}`; - } + const fixedRange = violation.currentEndYear + ? `${violation.currentStartYear}-${violation.expectedEndYear}` + : `${violation.expectedEndYear}`; sh.sed('-i', RegExp('Copyright \\([cC]\\) ' + currentRange), `Copyright (c) ${fixedRange}`, violation.file); }); @@ -337,19 +253,17 @@ function fixViolations(rootDir: string, violations: DateValidationResult[], opti // Helper types interface ValidationResult { file: string; - severity: Severity; violation: Violation; } interface DateValidationResult extends ValidationResult { currentStartYear: number; - expectedStartYear: number; currentEndYear?: number; expectedEndYear: number; } function isDateValidationResult(object: ValidationResult): object is DateValidationResult { - return 'currentStartYear' in object && 'expectedStartYear' in object && 'expectedEndYear' in object; + return 'currentStartYear' in object && 'expectedEndYear' in object; } -type Violation = 'none' | 'noOrMissingHeader' | 'incorrectCopyrightPeriod' | 'invalidCopyrightYear'; +type Violation = 'none' | 'noOrMissingHeader' | 'invalidEndYear'; diff --git a/dev-packages/cli/src/commands/release/release-vscode-integration.ts b/dev-packages/cli/src/commands/release/release-vscode-integration.ts index 0dee142..86d400a 100644 --- a/dev-packages/cli/src/commands/release/release-vscode-integration.ts +++ b/dev-packages/cli/src/commands/release/release-vscode-integration.ts @@ -51,7 +51,8 @@ function updateExternalGLSPDependencies(version: string): void { { name: '@eclipse-glsp/protocol', version }, { name: '@eclipse-glsp/client', version }, { name: '@eclipse-glsp-examples/workflow-glsp', version }, - { name: '@eclipse-glsp-examples/workflow-server', version } + { name: '@eclipse-glsp-examples/workflow-server', version }, + { name: '@eclipse-glsp-examples/workflow-server-bundled', version } ); } diff --git a/dev-packages/cli/src/util/git-util.ts b/dev-packages/cli/src/util/git-util.ts index d8748df..13a1f07 100644 --- a/dev-packages/cli/src/util/git-util.ts +++ b/dev-packages/cli/src/util/git-util.ts @@ -84,28 +84,6 @@ export function getLastModificationDate(filePath?: string, repoRoot?: string, ex } return new Date(result.stdout.trim()); } -/** - * Returns the last modification date of a file in a git repo. - * @param filePath The file - * @param repoRoot The path to the repo root. If undefined the current working directory is used. - * @param excludeMessage Only consider commits that don`t match the excludeMessage - * @returns The date or undefined if the file is outside of the git repo. - */ -export function getFirstModificationDate(filePath: string, repoRoot?: string, excludeMessage?: string): Date | undefined { - cdIfPresent(repoRoot); - const additionalArgs = excludeMessage ? `--grep="${excludeMessage}" --invert-grep` : ''; - const result = sh.exec(`git log ${additionalArgs} --pretty="format:%ci" --follow ${filePath}`, getShellConfig()); - if (result.code !== 0) { - return undefined; - } - const datesString = result.stdout.trim(); - if (datesString.length === 0) { - return new Date(); - } - - const date = datesString.split('\n').pop(); - return date ? new Date(date) : undefined; -} export function getFilesOfCommit(commitHash: string, repoRoot?: string): string[] { cdIfPresent(repoRoot); @@ -117,38 +95,6 @@ export function getFilesOfCommit(commitHash: string, repoRoot?: string): string[ return result.stdout.trim().split('\n'); } -/** - * Returns the commit hash of the initial commit of the given repository - * @param repoRoot The path to the repo root. If undefined the current working directory is used. - * @returns The commit hash or undefined if something went wrong. - */ -export function getInitialCommit(repoRoot?: string): string | undefined { - cdIfPresent(repoRoot); - const result = sh.exec('git log --pretty=oneline --reverse', getShellConfig()); - if (result.code !== 0) { - return undefined; - } - const commits = result.stdout.trim(); - if (commits.length === 0) { - return undefined; - } - return commits.substring(0, commits.indexOf(' ')); -} - -/** - * Returns the commit hash of the first commit for a given file (across renames). - * @param repoRoot The path to the repo root. If undefined the current working directory is used. - * @returns The commit hash or undefined if something went wrong. - */ -export function getFirstCommit(filePath: string, repoRoot?: string): string | undefined { - cdIfPresent(repoRoot); - const result = sh.exec(`git log --follow --pretty=format:"%H" ${filePath}`, getShellConfig()); - if (result.code !== 0) { - return undefined; - } - return result.stdout.trim().split('\n').pop(); -} - export function getLatestGithubRelease(path?: string): string { cdIfPresent(path); const release = sh.exec('gh release list --exclude-drafts -L 1', getShellConfig()).stdout.trim().split('\t'); diff --git a/dev-packages/config-test/package.json b/dev-packages/config-test/package.json index 87f0ca7..2a3cd65 100644 --- a/dev-packages/config-test/package.json +++ b/dev-packages/config-test/package.json @@ -1,6 +1,6 @@ { "name": "@eclipse-glsp/config-test", - "version": "2.0.0", + "version": "2.2.0-next", "description": "Meta package that provides Mocha and nyc configurations for GLSP projects", "keywords": [ "eclipse", @@ -25,8 +25,8 @@ } ], "dependencies": { - "@eclipse-glsp/mocha-config": "~2.0.0", - "@eclipse-glsp/nyc-config": "~2.0.0", + "@eclipse-glsp/mocha-config": "2.2.0-next", + "@eclipse-glsp/nyc-config": "2.2.0-next", "@istanbuljs/nyc-config-typescript": "^1.0.2", "@types/chai": "^4.3.7", "@types/mocha": "^10.0.2", diff --git a/dev-packages/config/package.json b/dev-packages/config/package.json index b79c090..035bb91 100644 --- a/dev-packages/config/package.json +++ b/dev-packages/config/package.json @@ -1,6 +1,6 @@ { "name": "@eclipse-glsp/config", - "version": "2.0.0", + "version": "2.2.0-next", "description": "Meta package that provides Typescript, eslint and prettier configurations and common dev dependencies for GLSP projects", "keywords": [ "eclipse", @@ -24,9 +24,9 @@ } ], "dependencies": { - "@eclipse-glsp/eslint-config": "~2.0.0", - "@eclipse-glsp/prettier-config": "~2.0.0", - "@eclipse-glsp/ts-config": "~2.0.0", + "@eclipse-glsp/eslint-config": "2.2.0-next", + "@eclipse-glsp/prettier-config": "2.2.0-next", + "@eclipse-glsp/ts-config": "2.2.0-next", "@typescript-eslint/eslint-plugin": "^6.7.5", "@typescript-eslint/parser": "^6.7.5", "eslint": "^8.51.0", diff --git a/dev-packages/dev/package.json b/dev-packages/dev/package.json index 6234f9d..8d1dcbc 100644 --- a/dev-packages/dev/package.json +++ b/dev-packages/dev/package.json @@ -1,6 +1,6 @@ { "name": "@eclipse-glsp/dev", - "version": "2.0.0", + "version": "2.2.0-next", "description": "All-in-one meta package that provides the GLSP development and test configuration packages, as well as the GLSP CLI package", "keywords": [ "eclipse", @@ -24,9 +24,9 @@ } ], "dependencies": { - "@eclipse-glsp/cli": "~2.0.0", - "@eclipse-glsp/config": "~2.0.0", - "@eclipse-glsp/config-test": "~2.0.0" + "@eclipse-glsp/cli": "2.2.0-next", + "@eclipse-glsp/config": "2.2.0-next", + "@eclipse-glsp/config-test": "2.2.0-next" }, "publishConfig": { "access": "public" diff --git a/dev-packages/eslint-config/package.json b/dev-packages/eslint-config/package.json index 6165b4d..b2a2af3 100644 --- a/dev-packages/eslint-config/package.json +++ b/dev-packages/eslint-config/package.json @@ -1,6 +1,6 @@ { "name": "@eclipse-glsp/eslint-config", - "version": "2.0.0", + "version": "2.2.0-next", "description": "Shared ESLint configuration for GLSP projects", "keywords": [ "eclipse", diff --git a/dev-packages/mocha-config/package.json b/dev-packages/mocha-config/package.json index 4eb4961..426c423 100644 --- a/dev-packages/mocha-config/package.json +++ b/dev-packages/mocha-config/package.json @@ -1,6 +1,6 @@ { "name": "@eclipse-glsp/mocha-config", - "version": "2.0.0", + "version": "2.2.0-next", "description": "Shared Mocha test configuration for GLSP projects", "keywords": [ "eclipse", diff --git a/dev-packages/nyc-config/package.json b/dev-packages/nyc-config/package.json index 784f40a..9a6c4f7 100644 --- a/dev-packages/nyc-config/package.json +++ b/dev-packages/nyc-config/package.json @@ -1,6 +1,6 @@ { "name": "@eclipse-glsp/nyc-config", - "version": "2.0.0", + "version": "2.2.0-next", "description": "Shared nyc configuration for GLSP projects", "keywords": [ "eclipse", diff --git a/dev-packages/prettier-config/package.json b/dev-packages/prettier-config/package.json index b86feab..3f40489 100644 --- a/dev-packages/prettier-config/package.json +++ b/dev-packages/prettier-config/package.json @@ -1,6 +1,6 @@ { "name": "@eclipse-glsp/prettier-config", - "version": "2.0.0", + "version": "2.2.0-next", "description": "Shared Prettier configuration for GLSP projects", "keywords": [ "eclipse", diff --git a/dev-packages/ts-config/package.json b/dev-packages/ts-config/package.json index 6eef59e..9014dbf 100644 --- a/dev-packages/ts-config/package.json +++ b/dev-packages/ts-config/package.json @@ -1,6 +1,6 @@ { "name": "@eclipse-glsp/ts-config", - "version": "2.0.0", + "version": "2.2.0-next", "description": "Shared Typescript configuration for GLSP projects", "keywords": [ "eclipse", diff --git a/lerna.json b/lerna.json index a5528fa..ef0fe25 100644 --- a/lerna.json +++ b/lerna.json @@ -1,5 +1,5 @@ { - "version": "2.0.0", + "version": "2.2.0-next", "useWorkspaces": true, "npmClient": "yarn", "command": { diff --git a/package.json b/package.json index 4e073e9..db9ce7f 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "parent", - "version": "2.0.0", + "version": "2.2.0-next", "private": true, "workspaces": [ "dev-packages/*"