Invalid/unused `pants: ...` directives like `no-infer-dep` are silently ignored

[huonw]

Describe the bug

Someone can currently write a # pants: no-infer-dep line that does nothing and receives no prompt for this. This may lead to users shipping code with unintended dependencies.

For instance, maybe one has a use_bar function in foo.py that does a lazy import of some chunky library bar. The default Pants behaviour will infer a dependency from foo to bar. However, if bar is chunky (or has chunky deps) one might not want to pull it in for all users of foo, only those that call use_bar (and those specific consumers can annotate the dependency manually).

# foo.py
def use_bar():
    import bar

One might make various mistakes that mean Pants still infers a dependency on bar:

def use_bar():
    # pants: no-infer-dep
    import bar

(The correct syntax is import bar # pants: no-infer-dep on one line. (Experienced Python users may know this is the usual convention, but other ecosystems have different conventions for this sort of comment, so may be tripped up.)

def use_bar():
    import bar # pants: no-infer

(Spelled incorrectly, lots of other variations of mistake too pants: no-dep or pants: no-inefr-dep.)

A user "should" validate that their code does what's expected, but we can help when it seems obviously-wrong. (And it's definitely possible to end up with this accidentally, e.g. add a correct comment, then a year later, apply a code-formatter that makes a large number of changes and happens to moves the comments: only a super-hero would remember to check the pants: no-infer-dep comments.)

Suggestion

Emit errors/warnings about pants: ... comments that don't apply, either because:

• directive not understood
• directive not attached to an appropriate element

Reproducer

cd $(mktemp -d)

cat > pants.toml <<EOF
[GLOBAL]
pants_version = "2.22.0a0"
backend_packages = ["pants.backend.python"]

[python]
interpreter_constraints = ["==3.10.*"]
EOF

echo 'python_sources(name="src")' > BUILD

cat > foo.py <<EOF
def use_bar():
    # pants: no-infer-dep
    import bar
EOF

touch bar.py

# BUG: no errors/warnings, only output is: //bar.py:src
pants dependencies foo.py

Pants version

2.21.0

OS macOS

Additional info https://github.com/pantsbuild/pants/issues/20207 is related, in that the subtleties of placing a no-infer-dep correctly matter less if Pants will at least guide users.

[kaos]

Is this a bug, though? feels more feature to me.. :)

[lilatomic]

I think there are 2 aspects to this:

1. unrecognised pragmas : pretty easy, just search all files and check against a list of known pragmas (it's just no-infer-dep)
2. unused pragmas : It's surprisingly difficult to see if a particular pragma is used. We process the pragma inline with the dependency inference https://github.com/pantsbuild/pants/blob/a4fbfb8db2dfcb7d2872ba981285db0e81c48b2f/src/rust/engine/dep_inference/src/python/mod.rs#L114. We only have that single pragma, so it makes sense that we don't really have the machinery for processing them.

We could write a new AST traverser to check that the no-infer-dep pragmas only appear attached to imports, possibly leveraging the same code in the dependency inferer.

[huonw]

possibly leveraging the same code in the dependency inferer

Yeah, I think this is important, or else it'll be easy for them to get out of sync.

I had this vague idea that we could somehow collect the pragmas, and then have the dependency inference indicate when it uses one (i.e. check it off the list), and then there's a separate step that emits errors/warnings about any unused... but I haven't thought through any details about making that actually work.