Skip to content

Add warn-short-path-literals setting#13489

Merged
roberth merged 1 commit intoNixOS:masterfrom
oknyshuk:add-warn-short-path-literals
Jul 29, 2025
Merged

Add warn-short-path-literals setting#13489
roberth merged 1 commit intoNixOS:masterfrom
oknyshuk:add-warn-short-path-literals

Conversation

@oknyshuk
Copy link
Contributor

@oknyshuk oknyshuk commented Jul 17, 2025

Add warn-short-path-literals setting

Closes: #13374

Problem

Path literals that don't start with ./ or ../ (like foo/bar) can be confusing and less explicit than their prefixed counterparts. Even experienced Nix developers are sometimes unaware that bare path literals are valid syntax, leading to inconsistent code style and potential confusion about the role of path expressions.

Solution

This PR adds a new warn-short-path-literals setting to EvalSettings that emits warnings when encountering relative path literals that don't start with . or /. When enabled, expressions like foo/bar will suggest using ./foo/bar instead.

The implementation follows the same pattern as the existing no-url-literals experimental feature but uses a regular setting instead, as requested in the issue.

Examples

Without the setting (default behavior):

$ nix eval --expr 'foo/bar'
/current/path/foo/bar

With the setting enabled:

$ nix eval --expr 'foo/bar' --warn-short-path-literals
warning: Short path literal 'foo/bar' is deprecated. Use './foo/bar' instead.
         at «string»:1:1:
              1| foo/bar
               | ^
/current/path/foo/bar

P.S.: Sorry if I've got something wrong, it's my first contribution to OSS in a while, and first one to the Nix project. Thanks.

@oknyshuk oknyshuk requested a review from edolstra as a code owner July 17, 2025 12:19
@roberth
Copy link
Member

roberth commented Jul 24, 2025

We'd much appreciate a test case as well. I see that no-url-literals is not tested unfortunately. The way we'd do it is to add it to tests/functional.
Could you help with that?

@oknyshuk
Copy link
Contributor Author

Could you help with that?

Never done tests in Meson, but I can try to. Should I commit the test for warn-short-path-literals here and the one for no-url-literals in the issue You opened?

@oknyshuk oknyshuk force-pushed the add-warn-short-path-literals branch from 216cea4 to 2b6822e Compare July 24, 2025 18:27
@roberth
Copy link
Member

roberth commented Jul 24, 2025

@k1gen Sounds good to me.

Unrelated: looks like the formatter needs to be run on the current diff.

@oknyshuk oknyshuk force-pushed the add-warn-short-path-literals branch from 2b6822e to e4e7369 Compare July 24, 2025 21:10
@github-actions github-actions bot added the with-tests Issues related to testing. PRs with tests have some priority label Jul 24, 2025
@oknyshuk
Copy link
Contributor Author

Unrelated: looks like the formatter needs to be run on the current diff.

Yeah, my bad. Thought I did that...

How does the test look?

@oknyshuk
Copy link
Contributor Author

oknyshuk commented Jul 24, 2025

Oops. Works on my machine:

[olk@think:~/dev/nix/build]$ ../outputs/out/bin/nix eval --expr 'test/subdir'
/home/olk/dev/nix/build/test/subdir
[olk@think:~/dev/nix]$ checkPhase
...
 65/205 nix-functional-tests:main / short-path-literals                              OK              0.84s
...
Ok:                 199
Fail:               0

Any ideas, @roberth?

@oknyshuk oknyshuk force-pushed the add-warn-short-path-literals branch from e4e7369 to deb116e Compare July 25, 2025 12:41
@oknyshuk
Copy link
Contributor Author

The test passes even if I don't create and populate test directories, should I remove this part in order not to litter in the tests dir?

# Create test directories
mkdir -p "$TEST_ROOT/test/subdir"
echo "test content" > "$TEST_ROOT/test/file.txt"

@roberth
Copy link
Member

roberth commented Jul 29, 2025

should I remove this part

That'd be good, thanks!

Add a new setting to warn about path literals that don't start with "." or "/". When enabled,
expressions like `foo/bar` will emit a warning suggesting to use `./foo/bar` instead.

A functional test is included.

The setting defaults to false for backward compatibility but could eventually default to true in
the future.

Closes: #13374

Co-authored-by: Robert Hensing <roberth@users.noreply.github.com>
@oknyshuk oknyshuk force-pushed the add-warn-short-path-literals branch from deb116e to 6d46dc9 Compare July 29, 2025 13:48
@roberth roberth merged commit c85a014 into NixOS:master Jul 29, 2025
14 checks passed
@roberth
Copy link
Member

roberth commented Jul 29, 2025

Thanks again!

@nixos-discourse
Copy link

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/nix-2-31-0-released/68465/1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

with-tests Issues related to testing. PRs with tests have some priority

Projects

None yet

Development

Successfully merging this pull request may close these issues.

No short path literals

3 participants