From cda00cd72ea762b30b89a9111ed75e235e5b4047 Mon Sep 17 00:00:00 2001 From: Dinh Le Date: Thu, 1 Oct 2026 10:17:18 +0700 Subject: [PATCH] fix(uncheck): reject paths outside cwd, take existing exclusions literally, skip linked folders - A file, folder or glob above the working directory now fails with a clear error instead of reaching oxlint and oxfmt as a "../" path they reject. - An exclusion naming an existing path leaves out that path alone, so `![id].ts` no longer also drops i.ts and d.ts; backslash escapes now reach the glob matcher on POSIX. - Linked folders, which git lists as one file, are no longer handed to the tools, so their files are not reported twice or fixed outside the repo. --- packages/uncheck/src/files.ts | 31 +++++++++++--- .../tests/uncheck/paths-exclusions.test.ts | 13 +++++- .../uncheck/tests/uncheck/paths-git.test.ts | 34 ++++++++++----- .../uncheck/tests/uncheck/paths-globs.test.ts | 8 ++++ .../uncheck/tests/uncheck/paths-tools.test.ts | 34 +-------------- packages/uncheck/tests/uncheck/paths.test.ts | 42 ++++++++++++------- .../tests/uncheck/tsc-references.test.ts | 8 ---- 7 files changed, 97 insertions(+), 73 deletions(-) diff --git a/packages/uncheck/src/files.ts b/packages/uncheck/src/files.ts index 2c84ce8..16a9c8a 100644 --- a/packages/uncheck/src/files.ts +++ b/packages/uncheck/src/files.ts @@ -1,9 +1,10 @@ -import { existsSync } from 'node:fs' +import { existsSync, statSync } from 'node:fs' import { Effect, FileSystem, Path, Predicate } from 'effect' import type { ChildProcessSpawner } from 'effect/unstable/process' import { Minimatch } from 'minimatch' +import { userError } from './errors' import { gitPaths } from './git' export type ProjectFiles = Effect.Effect< @@ -59,8 +60,9 @@ export const resolvePaths = Effect.fn(function* ( ) { const fs = yield* FileSystem.FileSystem const path = yield* Path.Path + // A backslash is a glob escape on POSIX, never a separator. const relative = (pattern: string) => - path.relative(cwd, path.resolve(cwd, pattern)).replaceAll('\\', '/') + path.relative(cwd, path.resolve(cwd, pattern)).split(path.sep).join('/') const includes = patterns.filter((pattern) => !pattern.startsWith('!')) const matched = new Set() @@ -70,6 +72,13 @@ export const resolvePaths = Effect.fn(function* ( for (const pattern of includes.length > 0 ? includes : ['.']) { const target = relative(pattern) + // oxlint and oxfmt reject a path containing "..". + if (target === '..' || target.startsWith('../') || path.isAbsolute(target)) { + return yield* userError( + `${pattern} is outside ${cwd}, run from a folder that contains it or pass one with --cwd`, + ) + } + // An existing path is taken as it is, so `app/[id].ts` names that file rather than a glob. const kind = yield* fs.stat(path.resolve(cwd, pattern)).pipe( Effect.map((info) => info.type), @@ -111,15 +120,18 @@ export const resolvePaths = Effect.fn(function* ( return () => true } - const matches = glob(target) + // As for an inclusion, so `![id].ts` leaves out that file and not `i.ts` too. + if (existsSync(path.resolve(cwd, pattern.slice(1)))) { + return (file: string) => file === target || file.startsWith(`${target}/`) + } - return (file: string) => file === target || file.startsWith(`${target}/`) || matches(file) + return glob(target) }) // One fiber per file costs far more than the check itself on a large project. const files = yield* Effect.sync(() => [...matched].filter( - (file) => !excludes.some((excluded) => excluded(file)) && existsSync(path.resolve(cwd, file)), + (file) => !excludes.some((excluded) => excluded(file)) && isFile(path.resolve(cwd, file)), ), ) @@ -128,6 +140,15 @@ export const resolvePaths = Effect.fn(function* ( const GLOB_CHARACTERS = /[*?[\]{}()]/ +// git lists a linked folder as one file, which the tools would check through the link. +function isFile(file: string): boolean { + try { + return statSync(file).isFile() + } catch { + return false + } +} + /** Dot files match too, as they do for oxfmt and for a directory given as it is. */ function glob(pattern: string): (file: string) => boolean { // Level 2 drops the `.` of `src/{.,deep}/*.ts` as path.matchesGlob does. diff --git a/packages/uncheck/tests/uncheck/paths-exclusions.test.ts b/packages/uncheck/tests/uncheck/paths-exclusions.test.ts index 4d0a301..d8fa3e1 100644 --- a/packages/uncheck/tests/uncheck/paths-exclusions.test.ts +++ b/packages/uncheck/tests/uncheck/paths-exclusions.test.ts @@ -45,9 +45,20 @@ describe.each(LAYOUTS)('uncheck with exclusions in a $name', ({ create, app }) = ) }) - it('leaves out a file whose name looks like a glob along with every file that glob matches', async () => { + it('leaves out an existing file or folder whose name looks like a glob as that path alone', async () => { const project = routesProject() + expect(await check(project, '.', '![id].ts')).toEqual(without('[id].ts')) + expect(await check(project, '.', '!(group)')).toEqual(without('(group)/page.ts')) + }) + + it('reads an exclusion naming no existing path as a glob, with \\ escaping', async () => { + const project = routesProject() + + expect(await check(project, '.', '!\\[id\\].ts')).toEqual(without('[id].ts')) + + project.write({ [`${cwd}/[id].ts`]: null }) + expect(await check(project, '.', '![id].ts')).toEqual(without('[id].ts', 'd.ts', 'i.ts')) }) diff --git a/packages/uncheck/tests/uncheck/paths-git.test.ts b/packages/uncheck/tests/uncheck/paths-git.test.ts index f8628a8..b648a4c 100644 --- a/packages/uncheck/tests/uncheck/paths-git.test.ts +++ b/packages/uncheck/tests/uncheck/paths-git.test.ts @@ -1,4 +1,4 @@ -import { writeFileSync } from 'node:fs' +import { readFileSync, writeFileSync } from 'node:fs' import { join } from 'node:path' import { cliError, LAYOUTS, temporaryDirectory } from '../utils/project' @@ -94,24 +94,38 @@ describe.each(LAYOUTS)('uncheck with paths in the git repository of a $name', ({ } }) - it('hands the tools a linked folder as the file git lists, so oxlint checks its files twice', async () => { - const project = create({ [`${routes}/shared/util.ts`]: CODE_WITH_VAR }) + it('checks a linked file but never a linked folder or a broken link, as the walk outside git', async () => { + const store = temporaryDirectory() + writeFileSync(join(store, 'vendor.ts'), CODE_WITH_VAR) + const project = create({ + [`${routes}/home.ts`]: CLEAN_CODE, + [`${routes}/shared/util.ts`]: CODE_WITH_VAR, + }) + .link(`${routes}/alias.ts`, 'home.ts') + .link(`${routes}/broken.ts`, 'missing.ts') .link(`${routes}/linked`, 'shared') + .link(`${routes}/vendor`, store) .commit() - const { exitCode, stdout } = await project.uncheck(['--only=oxlint', routes]) + const check = await project.uncheck(['--only=oxlint', routes]) - expect(exitCode).toBe(1) - expect(stdout).toContain(`${routes}/linked/util.ts:1:1`) - expect(stdout).toContain(`${routes}/shared/util.ts:1:1`) - expect(stdout).toContain('eslint(no-var)') - expect(selectedReport(stdout)).toEqual([ + expect(check.exitCode).toBe(1) + expect(check.stdout).toContain(`${routes}/shared/util.ts:1:1`) + expect(check.stdout).not.toContain(`${routes}/linked/`) + expect(check.stdout).not.toContain(`${routes}/vendor/`) + expect(selectedReport(check.stdout)).toEqual([ `uncheck in ${project.dir}`, - `▶ oxlint --no-error-on-unmatched-pattern ${routes}/linked ${routes}/shared/util.ts`, + `▶ oxlint --no-error-on-unmatched-pattern ${routes}/alias.ts ${routes}/home.ts ${routes}/shared/util.ts`, '✘ oxlint failed', '✘ 1 of 1 checks failed: oxlint', ' rerun with `--fix` to apply oxlint fixes', ]) + + const fix = await project.uncheck(['--only=oxlint', '--fix', routes]) + + expect(fix.exitCode).toBe(0) + expect(project.read(`${routes}/shared/util.ts`)).toBe('const count = 1;\nexport { count };\n') + expect(readFileSync(join(store, 'vendor.ts'), 'utf8')).toBe(CODE_WITH_VAR) }) it('leaves out a linked node_modules that a folder-only ignore rule misses', async () => { diff --git a/packages/uncheck/tests/uncheck/paths-globs.test.ts b/packages/uncheck/tests/uncheck/paths-globs.test.ts index bc8af90..b4bd758 100644 --- a/packages/uncheck/tests/uncheck/paths-globs.test.ts +++ b/packages/uncheck/tests/uncheck/paths-globs.test.ts @@ -102,6 +102,14 @@ describe.each(LAYOUTS)('uncheck with globs in a $name', ({ create, app }) => { expect(await check(project, routes('@(home|about).ts'))).toEqual(routes('about.ts', 'home.ts')) }) + it('matches a character escaped with \\ literally', async () => { + const project = routesProject() + + expect(await check(project, routes('\\[id\\].ts', '\\[slug\\]/*'))).toEqual( + routes('[id].ts', '[slug]/page.ts'), + ) + }) + it('reads a glob starting with "#" as a glob rather than a comment', async () => { const project = routesProject() diff --git a/packages/uncheck/tests/uncheck/paths-tools.test.ts b/packages/uncheck/tests/uncheck/paths-tools.test.ts index 87f9d6b..4ea4638 100644 --- a/packages/uncheck/tests/uncheck/paths-tools.test.ts +++ b/packages/uncheck/tests/uncheck/paths-tools.test.ts @@ -1,7 +1,4 @@ -import { writeFileSync } from 'node:fs' -import { join, relative } from 'node:path' - -import { LAYOUTS, report, temporaryDirectory } from '../utils/project' +import { LAYOUTS, report } from '../utils/project' import { CLEAN_CODE, CODE_WITH_VAR, NOT_COVERED, selectedReport } from './utils' const ONLY_FILE_CHECKS = ['--only=oxlint', '--only=oxfmt'] @@ -200,33 +197,4 @@ describe.each(LAYOUTS)('uncheck handing files to the tools in a $name', ({ creat expect(project.read(`${app}-draft.ts`)).toBe('const count = 1;\nexport { count };\n') expect(project.read(`${app}!notes.ts`)).toBe('export const notes = 1;\n') }) - - it('hands oxlint and oxfmt a file above the directory it runs in as a "../" path they reject', async () => { - const project = create() - const outside = temporaryDirectory() - writeFileSync(join(outside, 'shared.ts'), CLEAN_CODE) - const handed = relative(project.dir, join(outside, 'shared.ts')) - - const { exitCode, stdout, stderr } = await project.uncheck([ - ...ONLY_FILE_CHECKS, - join(outside, 'shared.ts'), - ]) - - expect(stderr).toBe('') - expect(exitCode).toBe(1) - expect(selectedReport(stdout)).toEqual([ - `uncheck in ${project.dir}`, - `▶ oxlint --no-error-on-unmatched-pattern ${handed}`, - '✘ oxlint failed', - `▶ oxfmt --check --no-error-on-unmatched-pattern ${handed}`, - '✘ oxfmt failed', - '✘ 2 of 2 checks failed: oxlint, oxfmt', - ' rerun with `--fix` to apply oxlint and oxfmt fixes', - ]) - expect( - stdout - .split('\n') - .filter((line) => line === `Error: \`${handed}\`: PATH must not contain ".."`), - ).toHaveLength(2) - }) }) diff --git a/packages/uncheck/tests/uncheck/paths.test.ts b/packages/uncheck/tests/uncheck/paths.test.ts index 2d4c1da..c4807c2 100644 --- a/packages/uncheck/tests/uncheck/paths.test.ts +++ b/packages/uncheck/tests/uncheck/paths.test.ts @@ -101,26 +101,36 @@ describe.each(LAYOUTS)('uncheck selecting files by path in a $name', ({ create, ]) }) - it('matches nothing with a directory or glob above the directory it runs in', async () => { + it('fails naming a file, directory or glob above the directory it runs in', async () => { const project = create({ [`${app}src/routes/home.ts`]: CLEAN_CODE, [`${app}src/legacy.ts`]: CODE_WITH_VAR, }) - const outside = temporaryDirectory() - writeFileSync(join(outside, 'shared.ts'), CLEAN_CODE) - - const { exitCode, stdout, stderr } = await project.uncheck( - ['--only=oxlint', '..', '../*.ts', outside], - { cwd: `${app}src/routes` }, - ) - - expect(exitCode).toBe(1) - expect(report(stdout)).toEqual([`uncheck in ${project.path(app, 'src/routes')}`]) - expect(stderr).toBe( - cliError( - `No files match .., ../*.ts, ${outside}. Pass --no-error-on-unmatched-pattern to run with whatever matched.`, - ), - ) + const outside = join(temporaryDirectory(), 'shared.ts') + writeFileSync(outside, CLEAN_CODE) + const routes = `${app}src/routes` + + for (const pattern of ['../legacy.ts', '..', '../*.ts', outside]) { + const fromInside = await project.uncheck(['--only=oxlint', 'home.ts', pattern], { + cwd: routes, + }) + const withCwd = await project.uncheck([ + '--only=oxlint', + '--no-error-on-unmatched-pattern', + `--cwd=${routes}`, + pattern, + ]) + + for (const { exitCode, stdout, stderr } of [fromInside, withCwd]) { + expect(exitCode).toBe(1) + expect(report(stdout)).toEqual([`uncheck in ${project.path(routes)}`]) + expect(stderr).toBe( + cliError( + `${pattern} is outside ${project.path(routes)}, run from a folder that contains it or pass one with --cwd`, + ), + ) + } + } }) }) diff --git a/packages/uncheck/tests/uncheck/tsc-references.test.ts b/packages/uncheck/tests/uncheck/tsc-references.test.ts index 1c83fba..eb71e0b 100644 --- a/packages/uncheck/tests/uncheck/tsc-references.test.ts +++ b/packages/uncheck/tests/uncheck/tsc-references.test.ts @@ -173,14 +173,6 @@ describe('tsc project references across the packages of a monorepo', () => { ) }) - it('builds a package from its folder for a change in a package it references', async () => { - const project = withFakeTsc(monorepo) - - expect(await tscPlan(project, 'packages/app', ['../core/src/index.ts'])).toEqual([ - '▶ tsc -b tsconfig.json', - ]) - }) - it('checks a package on its own from its folder', async () => { const project = withFakeTsc(monorepo)