diff --git a/dev-packages/cli/src/commands/check-header.ts b/dev-packages/cli/src/commands/check-header.ts index 3d7e3cf..38bffc1 100644 --- a/dev-packages/cli/src/commands/check-header.ts +++ b/dev-packages/cli/src/commands/check-header.ts @@ -24,9 +24,9 @@ import { configureExec, exec, filterFiles, + getChangesComparedToDefaultBranch, getChangesOfLastCommit, getLastModificationDate, - getUncommittedChanges, globby, readFile, replaceInFile, @@ -59,8 +59,9 @@ export const CheckHeaderCommand = baseCommand() // .addOption( new Option( '-t, --type ', - 'The scope of the check. In addition to a full recursive check, is also possible to only' + - ' consider pending changes or the last commit' + 'The scope of the check. In addition to a full recursive check, it is also possible to only consider' + + ' the files changed compared to the default branch (`changes`, incl. uncommitted changes - i.e. the' + + ' files that would show up in a pull request) or the files changed with the last commit (`lastCommit`)' ) .choices(checkTypes) .default('full') @@ -111,7 +112,7 @@ async function getFiles(rootDir: string, options: HeaderCheckOptions): Promise { diff --git a/dev-packages/cli/src/commands/repo/common/utils.ts b/dev-packages/cli/src/commands/repo/common/utils.ts index 844fc17..a374283 100644 --- a/dev-packages/cli/src/commands/repo/common/utils.ts +++ b/dev-packages/cli/src/commands/repo/common/utils.ts @@ -62,6 +62,23 @@ export function resolveWorkspaceDir(cliDir?: string): string { return process.cwd(); } +/** + * Resolves the absolute path to a specific repository within the workspace. + * + * Accepts a `--dir` that points at the workspace (the common case) as well as one that already + * points at the repository itself, so that callers passing `--dir ` don't end up with the + * repository segment duplicated (e.g. `.../glsp-server-node/glsp-server-node`). + */ +export function resolveRepoDir(repo: GLSPRepo, cliDir?: string): string { + const workspaceDir = resolveWorkspaceDir(cliDir); + const repoDir = path.resolve(workspaceDir, repo); + // `--dir` already points at the repo itself: don't append the repo name a second time. + if (!fs.existsSync(repoDir) && path.basename(workspaceDir) === repo) { + return workspaceDir; + } + return repoDir; +} + // ── Repo discovery ──────────────────────────────────────────────────────── export function discoverRepos(dir: string): GLSPRepo[] { diff --git a/dev-packages/cli/src/commands/repo/server-node.ts b/dev-packages/cli/src/commands/repo/server-node.ts index 6b0b272..435db40 100644 --- a/dev-packages/cli/src/commands/repo/server-node.ts +++ b/dev-packages/cli/src/commands/repo/server-node.ts @@ -18,7 +18,7 @@ import * as fs from 'fs'; import * as path from 'path'; import { Command } from 'commander'; import { baseCommand } from '../../util'; -import { configureRepoEnv, resolveWorkspaceDir } from './common/utils'; +import { configureRepoEnv, resolveRepoDir } from './common/utils'; export const BROWSER_BUNDLE_PATH = 'examples/workflow-server-bundled-web/wf-glsp-server-webworker.js'; export const NODE_BUNDLE_PATH = 'examples/workflow-server-bundled/wf-glsp-server-node.js'; @@ -39,8 +39,7 @@ export const BrowserBundleCommand: Command = baseCommand() .action(async (_cmdOptions: unknown, thisCmd: Command) => { const cli = thisCmd.opts<{ dir?: string; verbose: boolean }>(); configureRepoEnv(cli); - const dir = resolveWorkspaceDir(cli.dir); - const repoDir = path.resolve(dir, 'glsp-server-node'); + const repoDir = resolveRepoDir('glsp-server-node', cli.dir); const bundlePath = resolveBundlePath(repoDir, BROWSER_BUNDLE_PATH, 'Browser bundle'); process.stdout.write(bundlePath); }); @@ -53,8 +52,7 @@ export const NodeBundleCommand: Command = baseCommand() .action(async (_cmdOptions: unknown, thisCmd: Command) => { const cli = thisCmd.opts<{ dir?: string; verbose: boolean }>(); configureRepoEnv(cli); - const dir = resolveWorkspaceDir(cli.dir); - const repoDir = path.resolve(dir, 'glsp-server-node'); + const repoDir = resolveRepoDir('glsp-server-node', cli.dir); const bundlePath = resolveBundlePath(repoDir, NODE_BUNDLE_PATH, 'Node server bundle'); process.stdout.write(bundlePath); }); diff --git a/dev-packages/cli/src/util/git-util.spec.ts b/dev-packages/cli/src/util/git-util.spec.ts index ad5910f..a1b4334 100644 --- a/dev-packages/cli/src/util/git-util.spec.ts +++ b/dev-packages/cli/src/util/git-util.spec.ts @@ -15,9 +15,17 @@ ********************************************************************************/ import { expect } from 'chai'; -import { createSandbox } from 'sinon'; +import { createSandbox, match } from 'sinon'; import * as processUtil from './process-util'; -import { commitChanges, getDefaultBranch, getLastModificationDate, getUncommittedChanges, hasChanges } from './git-util'; +import { + commitChanges, + getChangesComparedToDefaultBranch, + getDefaultBranch, + getDefaultBranchRef, + getLastModificationDate, + getUncommittedChanges, + hasChanges +} from './git-util'; const sandbox = createSandbox(); @@ -84,6 +92,44 @@ describe('git-util', () => { }); }); + describe('getDefaultBranchRef', () => { + it('should resolve the ref from origin/HEAD when available', () => { + execStub.withArgs(match(/symbolic-ref/)).returns('origin/develop'); + expect(getDefaultBranchRef('/repo')).to.equal('origin/develop'); + }); + + it('should fall back to the first existing candidate ref', () => { + execStub.withArgs(match(/symbolic-ref/)).returns(''); + execStub.withArgs(match('origin/main')).throws(new Error('unknown revision')); + execStub.withArgs(match('origin/master')).throws(new Error('unknown revision')); + execStub.withArgs(match(/--verify --quiet main/)).returns(''); + expect(getDefaultBranchRef('/repo')).to.equal('main'); + }); + + it('should return undefined when no default branch can be determined', () => { + execStub.throws(new Error('not a git repository')); + expect(getDefaultBranchRef('/repo')).to.be.undefined; + }); + }); + + describe('getChangesComparedToDefaultBranch', () => { + it('should merge committed and uncommitted changes and deduplicate', () => { + execStub.withArgs(match(/symbolic-ref/)).returns('origin/main'); + execStub.withArgs(match(/diff --name-only/)).returns('src/a.ts\nsrc/b.ts'); + execStub.withArgs(match(/status --porcelain/)).returns(' M src/b.ts\n?? src/c.ts'); + const result = getChangesComparedToDefaultBranch('/repo'); + expect(result).to.have.members(['/repo/src/a.ts', '/repo/src/b.ts', '/repo/src/c.ts']); + expect(result).to.have.length(3); + }); + + it('should only consider uncommitted changes when no default branch is found', () => { + execStub.throws(new Error('not a git repository')); + execStub.withArgs(match(/status --porcelain/)).returns(' M src/only.ts'); + const result = getChangesComparedToDefaultBranch('/repo'); + expect(result).to.deep.equal(['/repo/src/only.ts']); + }); + }); + describe('getLastModificationDate', () => { it('should return a Date for a valid date string', () => { execStub.returns('2024-01-15 10:30:00 +0000'); diff --git a/dev-packages/cli/src/util/git-util.ts b/dev-packages/cli/src/util/git-util.ts index 392541d..562cd79 100644 --- a/dev-packages/cli/src/util/git-util.ts +++ b/dev-packages/cli/src/util/git-util.ts @@ -67,6 +67,54 @@ export function getChangesOfLastCommit(path?: string): string[] { .map(file => resolve(path ?? process.cwd(), file)); } +/** + * Returns the files that have been changed compared to the default branch (e.g. master/main), i.e. the + * files that would show up as changed in a pull request. Includes both the committed changes of the + * current branch (relative to the merge-base with the default branch) and any uncommitted + * (staged, not staged or untracked) changes. + * + */ +export function getChangesComparedToDefaultBranch(path?: string): string[] { + const baseRef = getDefaultBranchRef(path); + const committedChanges = baseRef + ? exec(`git diff --name-only ${baseRef}...HEAD`, { cwd: path, silent: true }) + .split('\n') + .filter(value => value.trim().length !== 0) + .map(file => resolve(path ?? process.cwd(), file.trim())) + : []; + + return [...new Set([...committedChanges, ...getUncommittedChanges(path)])]; +} + +/** + * Resolves a git ref pointing to the tip of the default branch (e.g. master/main) that can be used for diffing. + * The lookup is performed offline and prefers the remote-tracking branch (origin/) over the local branch. + * @returns The resolved ref or `undefined` if no default branch could be determined. + */ +export function getDefaultBranchRef(path?: string): string | undefined { + // `origin/HEAD` is set up by `git clone` and points to the remote default branch + try { + const ref = exec('git symbolic-ref --short refs/remotes/origin/HEAD', { cwd: path, silent: true, fatal: false }); + if (ref) { + return ref; + } + } catch { + // not configured, fall through to the candidate lookup below + } + + const candidates = ['origin/main', 'origin/master', 'main', 'master']; + return candidates.find(ref => refExists(ref, path)); +} + +function refExists(ref: string, path?: string): boolean { + try { + exec(`git rev-parse --verify --quiet ${ref}`, { cwd: path, silent: true, fatal: false }); + return true; + } catch { + return false; + } +} + /** * Returns the commit message of the last commit * diff --git a/dev-packages/cli/tests/e2e/check-header.e2e.spec.ts b/dev-packages/cli/tests/e2e/check-header.e2e.spec.ts index 332348c..869ac82 100644 --- a/dev-packages/cli/tests/e2e/check-header.e2e.spec.ts +++ b/dev-packages/cli/tests/e2e/check-header.e2e.spec.ts @@ -90,6 +90,17 @@ describe('checkHeaders e2e', () => { expect(result.stdout).to.contain('1 copyright header violations'); }); + it('should check files changed against the default branch with --type changes', () => { + commitFile(repoDir, 'src/base.ts', VALID_HEADER + '\nconst x = 1;\n', 'add base file'); + // Branch off and add a file without a header - only this file should be checked. + execSync('git checkout -b feature', { cwd: repoDir }); + commitFile(repoDir, 'src/feature.ts', 'const y = 2;\n', 'add feature file'); + const result = runCli(['checkHeaders', repoDir, '--type', 'changes']); + expect(result.stdout).to.contain('Check copy right headers of 1 files'); + expect(result.stdout).to.contain('feature.ts'); + expect(result.stdout).to.not.contain('base.ts'); + }); + it('should only check last commit files with --type lastCommit', () => { commitFile(repoDir, 'src/old.ts', VALID_HEADER + '\nconst x = 1;\n', 'add old file'); commitFile(repoDir, 'src/new.ts', 'const y = 2;\n', 'add new file');