webpro-nl / knip

✂️ Find unused files, dependencies and exports in your JavaScript and TypeScript projects. Knip it before you ship it!
https://knip.dev
ISC License
6.77k stars 159 forks source link

Git-ignore rules inside a project should not match parts of the absolute path outside the project #678

Open DaAitch opened 4 months ago

DaAitch commented 4 months ago

Knip (5.19.0) may behave differently on different environments, depending on

During the block https://github.com/webpro-nl/knip/blob/7c9b6455d302193961816229c9d30ce0525a794f/packages/knip/src/util/globby.ts#L136 git ignore rules are translated to glob ignore rules. Rules are prefixed by ** which leads to the problem, that git ignore rules now may match outside the project.

Example:

# .gitignore
# ignore all files in builds folders in the project
builds/
// bug in knip

// doesn't match any file, because absolute path matches a gitignore rule
fastGlob(['src/index.ts'], {
  cwd: '/Users/xxx/builds/project',
  ignore: ['**/builds/**', '**/builds'] // generated from git-ignore
})
// maybe-fix in knip

fastGlob(['src/index.ts'], {
  cwd: '/Users/xxx/builds/project',
  ignore: ['/Users/xxx/builds/project/**/builds/**', '/Users/xxx/builds/project/**/builds'] // fix
})
webpro commented 4 months ago

Thanks for the bug report.

I wonder how it's possible the **/build pattern matching files outside the pattern influences the results? Any chance a small explanation or even reproduction could be set up?

That might be a reason to either improve Knip's docs and/or implementation: perhaps by extending or doing something similar to the source mapping (dist/index.jssrc/index.ts) that was recently introduced.

DaAitch commented 4 months ago

@webpro here it is: https://github.com/DaAitch/knip-issue-678

I simulated an absolute path, by checking in some folders.

The git-ignore rule builds/ in <PROJECT_ROOT>/Users/knipuser/cicd/builds/project42/.gitignore is translated to a glob-ignore for **/builds/**.

webpro commented 4 months ago

Thanks, that helped a lot.

I think I found the issue, it seems to originate here in fast-glob: https://github.com/mrmlnc/fast-glob/issues/441

Need to figure out whether it can/will be fixed downstream, or work around it in Knip.

DaAitch commented 4 months ago

@webpro Okay interesting. Here is a red knip unit test https://github.com/DaAitch/knip/commit/5989d4a12c8d4c0a61b5b849bf6a474745d60e5c to reproduce

webpro commented 4 months ago

Thanks, I'd prefer fixes and test coverage in the downstream lib, let's await that first: https://github.com/mrmlnc/fast-glob/pull/445