biome icon indicating copy to clipboard operation
biome copied to clipboard

🐛 2.0.0-beta.1: Prints file path as if they were absolute

Open mdevils opened this issue 8 months ago • 10 comments

Environment information

CLI:
  Version:                      2.0.0-beta.1
  Color support:                true

Platform:
  CPU Architecture:             aarch64
  OS:                           macos

Environment:
  BIOME_LOG_PATH:               unset
  BIOME_LOG_PREFIX_NAME:        unset
  BIOME_CONFIG_PATH:            unset
  NO_COLOR:                     unset
  TERM:                         xterm
  JS_RUNTIME_VERSION:           v20.11.0
  JS_RUNTIME_NAME:              node
  NODE_PACKAGE_MANAGER:         unset

Biome Configuration:
  Status:                       Loaded successfully
  Path:                         /Users/user/Projects/test-proj/biome.json
  Formatter enabled:            true
  Linter enabled:               true
  Assist enabled:               true
  VCS enabled:                  false

Workspace:
  Open Documents:               0

What happened?

I lint my project (via biome lint src) and here is the error message format I get:

/src/routes.tsx:128:35 lint/correctness/useHookAtTopLevel ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
  ✖ This hook is...

The problem is file path. It starts with a slash and terminal considers it to be an absolute path and fails to open the editor to edit that file.

In 1x version the path was correct.

Expected result

Here is how the errors were formatted in 1.9.4:

src/routes.tsx:65:64 lint/style/noNonNullAssertion  FIXABLE  ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━

  ✖ Forbidden non-null assertion.

Code of Conduct

  • [x] I agree to follow Biome's Code of Conduct

mdevils avatar Apr 03 '25 18:04 mdevils

This is a regression of the attempt of making paths relative.

@Conaclos what do you think, should we revert it or attempt to fix it?

ematipico avatar Apr 03 '25 18:04 ematipico

@Conaclos what do you think, should we revert it or attempt to fix it?

I am not sure we have a choice. We normalize paths to get proper glob matching...

Conaclos avatar Apr 03 '25 19:04 Conaclos

Could we just trim the leading slash off when printing it? Or is it more complicated than that?

dyc3 avatar Apr 03 '25 19:04 dyc3

@ematipico @Conaclos Here is a fix proposal: https://github.com/biomejs/biome/pull/5568

mdevils avatar Apr 03 '25 19:04 mdevils

It's more complex than that. If you run the CLI in verbose mode, we still emit diagnostics that have absolute paths. Those diagnostics are emitted during the traversal, and in those cases we need absolute paths.

I tried to compute relative paths, but that requires holding off the base directory during the crawling. It requires some refactor to achieve that, and it gets hairy

ematipico avatar Apr 03 '25 19:04 ematipico

@ematipico how do I reproduce the problem you are describing? I just didn't entirely understood what's the issue and how do I see the issue.

mdevils avatar Apr 03 '25 20:04 mdevils

src/routes.tsx:128:35 lint/correctness/useHookAtTopLevel ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━

  ✖ This hook is being called from a nested function, but all hooks must be called unconditionally from the top-level component.

    127 │
  > 128 │                 const hasData = useMemo(() => {

  ℹ For React to preserve state between calls, hooks needs to be called unconditionally and always in the same order.

  ℹ See https://reactjs.org/docs/hooks-rules.html#only-call-hooks-at-the-top-level


Checked 1 file in 30s. No fixes applied.
Found 9 errors.
 VERBOSE  ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━

  ℹ Files processed:

  - /Users/mdevils/Projects/test-proj/src/routes.tsx


 VERBOSE  ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━

  ℹ Files fixed:

  ⚠ The list is empty.

@ematipico This is how verbose output looks like after the fix. Do you mean that in case of verbose file paths (i.e. src/routes.tsx) should all be printed as absolute paths?

mdevils avatar Apr 03 '25 20:04 mdevils

@Conaclos what do you think, should we revert it or attempt to fix it?

I am not sure we have a choice. We normalize paths to get proper glob matching...

The paths that we pass to the diagnostics aren't the same paths we use for glob matching.

ematipico avatar Apr 04 '25 07:04 ematipico

@ematipico how do I reproduce the problem you are describing? I just didn't entirely understood what's the issue and how do I see the issue.

You have to run the CLI with --verbose, and run it inside a folder that contains files that Biome can't handle, for example .md, any image, etc.

ematipico avatar Apr 04 '25 07:04 ematipico

@ematipico here is another attempt to fix it. It's not ideal, but does the job also respecting the verbose flag.

I feel like there is some PrintContext object is missing where additional information about printing can be stored, such as verbose flag itself, along with working_directory and potentially other stuff. I'm not happy about fetching working directory in the printer to be honest.

mdevils avatar Apr 04 '25 20:04 mdevils