fix(uncheck): reject paths outside cwd, take existing exclusions literally, skip linked folders (#20)

- 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.

Author: dinwwwh

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<string>()
@@ -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.

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'))
   })
 

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 () => {

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()
 

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)
-  })
 })

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`,
+          ),
+        )
+      }
+    }
   })
 })
 

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)