search

mrtazz / checkmake

experimental linter/analyzer for Makefiles
MIT License
1.04k stars 45 forks source link

minphony false positives #87

Open mcandre opened 1 year ago

mcandre commented 1 year ago

Please don't enforce a PHONY declaration for tasks that are not declared in our Makefiles.

That is, if clean isn't even implemented, then it should not be marked PHONY. Same goes for test and all.

One could argue that all, test, and clean are idiomatic tasks that every Makefile should implement. While I personally useful default tasks, TDD, and practical cleanup tasks, even so, I think that would be an overbearing rule to inflict on users.

Hmm, looks like the minphony rule can't even be disabled by checkmake.ini? It is to cry.

MajorDallas commented 1 year ago

Falls under the title, but different from OP: I do have a required target and minphony is telling me that I do not.

Knowing nothing about how the parser works, the only thing I can think of that could be confusing it is that the phony target has a real target for a prerequisite. 

.PHONY : reqs container_rebuild container_update test test-now print
# ...
test : tests/.last_test
tests/.last_test : $(PY_SRC_FILES)
    cd tests; poetry run pytest . && touch .last_test
checkmake Makefile
    RULE              DESCRIPTION             FILE NAME   LINE NUMBER

  minphony   Missing required phony target    Makefile    0
             "all"
  minphony   Missing required phony target    Makefile    0
             "clean"
  minphony   Missing required phony target    Makefile    0
             "test"
mcandre commented 1 year ago

Plug:

https://github.com/mcandre/unmake

cooljeanius commented 12 months ago

PR #88 was meant to close issue #86, but I think it would also at least somewhat address this one, as well. See also issue #72 as well. Edit: and issue #15 too.