Skip to content

Add warn-short-path-literals setting#13489

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

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

Conversation

@oknyshuk

@oknyshuk oknyshuk commented Jul 17, 2025

Copy link
Copy Markdown
Contributor

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
Comment thread src/libexpr/parser.y Outdated
@roberth

roberth commented Jul 24, 2025

Copy link
Copy Markdown
Member

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
Copy Markdown
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?

@roberth

roberth commented Jul 24, 2025

Copy link
Copy Markdown
Member

@K1gen Sounds good to me.

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

@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
Copy Markdown
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

oknyshuk commented Jul 24, 2025

Copy link
Copy Markdown
Contributor Author

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?

Comment thread src/libexpr/include/nix/expr/eval-settings.hh Outdated
Comment thread tests/functional/short-path-literals.sh Outdated
@oknyshuk

Copy link
Copy Markdown
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

roberth commented Jul 29, 2025

Copy link
Copy Markdown
Member

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>
@roberth roberth merged commit c85a014 into NixOS:master Jul 29, 2025
14 checks passed
@roberth

roberth commented Jul 29, 2025

Copy link
Copy Markdown
Member

Thanks again!

@nixos-discourse

Copy link
Copy Markdown

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