Skip to content

Commit

Permalink
Refactor check-header script (#1289)
Browse files Browse the repository at this point in the history 
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`
  • Loading branch information
@tortmayr
tortmayr committed Mar 22, 2024
1 parent ec752a8 commit 886d2d0
Show file tree
Hide file tree
Showing 14 changed files with 63 additions and 201 deletions.
4 changes: 2 additions & 2 deletions dev-packages/cli/package.json
View file
@@ -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",
Expand Down Expand Up @@ -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",
Expand Down
167 changes: 41 additions & 126 deletions dev-packages/cli/src/commands/check-header.ts
View file
Expand Up @@ -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';
Expand All @@ -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('<rootDir>', 'The starting directory for the check', validateGitDirectory)
.addOption(
new Option(
Expand All @@ -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 <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);

Expand Down Expand Up @@ -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
Expand All @@ -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.
Expand All @@ -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);
});

Expand All @@ -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 {
Expand All @@ -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();
Expand All @@ -274,53 +203,41 @@ 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);
}

LOGGER.info('Check completed');
}

function toPrintMessage(result: ValidationResult): string {
const colors: Record<Severity, string> = {
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.currentStartYear}-${violation.expectedEndYear}`
: `${violation.expectedEndYear}`;

sh.sed('-i', RegExp('Copyright \\([cC]\\) ' + currentRange), `Copyright (c) ${fixedRange}`, violation.file);
});
Expand All @@ -337,19 +254,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';
3 changes: 2 additions & 1 deletion dev-packages/cli/src/commands/release/release-vscode-integration.ts
View file
Expand Up @@ -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 }
);
}

Expand Down
54 changes: 0 additions & 54 deletions dev-packages/cli/src/util/git-util.ts
View file
Expand Up @@ -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);
Expand All @@ -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');
Expand Down

0 comments on commit 886d2d0

Please sign in to comment.