dakrone / lein-bikeshed

A Leiningen plugin designed to tell you your code is bad, and that you should feel bad
176 stars 23 forks source link

Line length and ending blank line checks no longer work in 0.4.0 #30

Closed robert914 closed 7 years ago

robert914 commented 7 years ago

After updating my project from 0.3.0 of Bikeshed to 0.4.0, it no longer seems to correctly check the line length rules (80 char default) and the EOF blank line check.

Sample output from 0.3.0:

Checking for lines longer than 80 characters.
Badly formatted files:
/Users/robert/Downloads/hellworld/src/hellworld/core.clj:7:  (println "This line is too long and should fail 80 character limit but no longer does in 0.4.0 of Bikeshed")

Checking for lines with trailing whitespace.
No lines found.

Checking for files ending in blank lines.
Badly formatted files:
/Users/robert/Downloads/hellworld/src/hellworld/core.clj

Checking for redefined var roots in source directories.
No with-redefs found.

Checking whether you keep up with your docstrings.
0/1 [0.00%] namespaces have docstrings.
1/1 [100.00%] functions have docstrings.
Use -v to list namespaces/functions without docstrings

Checking for arguments colliding with clojure.core functions.
Subprocess failed

Sample output on same project in 0.4.0:

Checking for lines longer than 80 characters.
No lines found.

Checking for lines with trailing whitespace.
No lines found.

Checking for files ending in blank lines.
No files found.

Checking for redefined var roots in source directories.
No with-redefs found.

Checking whether you keep up with your docstrings.
0/1 [0.00%] namespaces have docstrings.
1/1 [100.00%] functions have docstrings.
Use -v to list namespaces/functions without docstrings

Checking for arguments colliding with clojure.core functions.

To reproduce, all you need to do is:

lein new app helloworld
# edit src/helloworld/core.clj
# add this line:
  (println "This line is too long and should fail 80 character limit but no longer does in 0.4.0 of Bikeshed")
# add a blank line to the end of the file

Please look into this, thanks!

dakrone commented 7 years ago

Hmm... I'm not able to reproduce this:

~/src/clj/hello λ rm -rf ~/.m2/repository/lein-bikeshed/lein-bikeshed/0.4.0/
~/src/clj/hello λ lein bikeshed -v
Retrieving lein-bikeshed/lein-bikeshed/0.4.0/lein-bikeshed-0.4.0.pom from clojars
Retrieving lein-bikeshed/lein-bikeshed/0.4.0/lein-bikeshed-0.4.0.jar from clojars

Checking for lines longer than 80 characters.
Badly formatted files:
/home/hinmanm/src/clj/hello/src/hello/core.clj:9:(println "This line is too long and should fail 80 character limit but no longer does in 0.4.0 of Bikeshed")

Checking for lines with trailing whitespace.
No lines found.

Checking for files ending in blank lines.
No files found.

Checking for redefined var roots in source directories.
No with-redefs found.

Checking whether you keep up with your docstrings.
This line is too long and should fail 80 character limit but no longer does in 0.4.0 of Bikeshed
0/1 [0.00%] namespaces have docstrings.
1/1 [100.00%] functions have docstrings.

Namespaces without docstrings:
hello.core

Methods without docstrings:

Checking for arguments colliding with clojure.core functions.
Subprocess failed
~/src/clj/hello λ cat project.clj 
(defproject hello "0.1.0-SNAPSHOT"
  :description "FIXME: write description"
  :url "http://example.com/FIXME"
  :license {:name "Eclipse Public License"
            :url "http://www.eclipse.org/legal/epl-v10.html"}
  :dependencies [[org.clojure/clojure "1.8.0"]]
  :main ^:skip-aot hello.core
  :target-path "target/%s"
  :profiles {:uberjar {:aot :all}}
  :plugins [[lein-bikeshed "0.4.0"]])

Is there any other difference in your project?

robert914 commented 7 years ago

I have this:

(defproject hellworld "0.1.0-SNAPSHOT"
  :description "FIXME: write description"
  :url "http://example.com/FIXME"
  :license {:name "Eclipse Public License"
            :url "http://www.eclipse.org/legal/epl-v10.html"}
  :dependencies [[org.clojure/clojure "1.8.0"]]
  :main ^:skip-aot hellworld.core
  :target-path "target/%s"
  :profiles {:uberjar {:aot :all}})

And I have this in my ~/.lein/profiles.clj

{:user {:plugins [[lein-ancient "0.6.10" :exclusions [org.clojure/clojure]]
                  ;[lein-bikeshed "0.3.0" :exclusions [org.clojure/clojure]]
                  [lein-bikeshed "0.4.0" :exclusions [org.clojure/clojure]]
                  [lein-cljfmt "0.5.6" :exclusions [org.clojure/clojure]]
                  [lein-cloverage "1.0.9" :exclusions [org.clojure/clojure]]
                  [lein-kibit "0.1.2" :exclusions [org.clojure/clojure]]
                  [lein-print "0.1.0" :exclusions [org.clojure/clojure]]
                  [jonase/eastwood "0.2.3" :exclusions [org.clojure/clojure]]
                  [venantius/ultra "0.5.0" :exclusions [org.clojure/clojure]]]
        ;:eastwood {:namespaces [:source-paths]}
        ;:dependencies [[pjstadig/humane-test-output "0.8.1"]]
        ;:injections [(require 'pjstadig.humane-test-output)
        ;             (pjstadig.humane-test-output/activate!)]
        :aliases {"checkdeps" ^{:doc "check all deps"} ["do"
                                                        ["deps" ":tree"]
                                                        ["ancient" "check" ":all" ":check-clojure"]
                                                        ["ancient" "check-profiles"]]
                  "showdeps" ^{:doc "show all deps"} ["do"
                                                      ["deps" ":tree"]
                                                      ["deps" ":plugin-tree"]
                                                      ["deps" ":implicits"]]
                  "lint" ^{:doc "check all code"} ["do"
                                                   ["cljfmt" "check"]
                                                   ["kibit"]
                                                   ["eastwood"]
                                                   ["bikeshed"]] ; "--verbose"]]
                  "format" "cljfmt"}}
 :repl {:source-paths ["/Users/robert/.lein/repl"]}}
dakrone commented 7 years ago

Can you try adding :plugins [[lein-bikeshed "0.4.0"]] to your project.clj (just to see if it's a problem with some other plugin overriding it) and then running:

rm -rf ~/.m2/repository/lein-bikeshed/lein-bikeshed/0.4.0/

And then try running it again? If it still doesn't work, can you zip/targz the project and attach it here?

robert914 commented 7 years ago

Ok, I did the following:

  1. Added ":plugins [[lein-bikeshed "0.4.0"]]" to the project.clj
  2. Removed entire repository with: rm -rf ~/.m2/repository
  3. Moved my profiles.clj aside: mv ~/.lein/profiles.clj ~/.lein/profiles.clj.save
  4. Ran bikeshed: lein bikeshed

Still no errors. Changed the bikeshed line to 0.3.0 in the project.clj and the errors appear.

Attaching project zip file.

robert914 commented 7 years ago

hellworld.zip

robert914 commented 7 years ago

Here's a zip of the repository after the wipe and run of bikeshed in that project as above. repository.zip

robert914 commented 7 years ago

Odd that you cannot reproduce this. My system setup for reference: Mac OS Sierra 10.12.1 MacBook Pro 15 Late-2013 Leiningen 2.7.1 on Java 1.8.0_112 Java HotSpot(TM) 64-Bit Server VM

dakrone commented 7 years ago

This is failing correctly for me:

~/Downloads/hellworld λ lein bikeshed

Checking for lines longer than 80 characters.
Badly formatted files:
/home/hinmanm/Downloads/hellworld/src/hellworld/core.clj:7:  (println "This line is too long and should fail 80 character limit but no longer does in 0.4.0 of Bikeshed")

Checking for lines with trailing whitespace.
No lines found.

Checking for files ending in blank lines.
Badly formatted files:
/home/hinmanm/Downloads/hellworld/src/hellworld/core.clj

Checking for redefined var roots in source directories.
No with-redefs found.

Checking whether you keep up with your docstrings.
0/1 [0.00%] namespaces have docstrings.
1/1 [100.00%] functions have docstrings.
Use -v to list namespaces/functions without docstrings

Checking for arguments colliding with clojure.core functions.
Subprocess failed

Can you try manually running these commands from within the project directory:

find src -regextype posix-extended -regex '.*\\.clj[scx]?' | xargs egrep -H -n '^.{80,}$'
find src -regextype posix-extended -regex ".*\\.clj[scx]?" | xargs egrep -H -n '^.{80,}$'
bash -c "find src -regextype posix-extended -regex '.*\\.clj[scx]?' | xargs egrep -H -n '^.{80,}$'"
robert914 commented 7 years ago

That's the problem, "-regextype" is not a valid option on Mac OS Sierra's BSD find command:

$ find src -regextype posix-extended -regex '.*\\.clj[scx]?' | xargs egrep -H -n '^.{80,}$'
find: -regextype: unknown primary or operator
$ find src -regextype posix-extended -regex ".*\\.clj[scx]?" | xargs egrep -H -n '^.{80,}$'
find: -regextype: unknown primary or operator
$ bash -c "find src -regextype posix-extended -regex '.*\\.clj[scx]?' | xargs egrep -H -n '^.{80,}$'"
find: -regextype: unknown primary or operator
dakrone commented 7 years ago

Ahh okay, that would explain why, I believe this used to work on OSX in the past.

dakrone commented 7 years ago

Does this work?

bash -c "/bin/find src -regex '.*\\.clj.*' | xargs egrep -H -n '^.{80,}$'"
robert914 commented 7 years ago

No, but /usr/bin/find does:

$ bash -c "/bin/find src -regex '.*\\.clj*' | xargs egrep -H -n '^.{80,}$'"
bash: /bin/find: No such file or directory
$ which find
/usr/bin/find
$ bash -c "/usr/bin/find src -regex '.*\\.clj*' | xargs egrep -H -n '^.{80,}$'"
src/hellworld/core.clj:7:  (println "This line is too long and should fail 80 character limit but no longer does in 0.4.0 of Bikeshed")
robert914 commented 7 years ago

Does this find command affect the extra EOF blank lines check as well? That is not working either.

dakrone commented 7 years ago

Does this find command affect the extra EOF blank lines check as well?

Yes, that also uses the same fine command.

No, but /usr/bin/find does:

Okay, that's fine, bikeshed calls whatever find is in the path, it doesn't hardcode /bin/find (I just did that on my machine)

dakrone commented 7 years ago

@robert914 one more question, does the following work for you:

bash -c "/usr/bin/find src -regex '.*\\.clj[scx]?' | xargs egrep -H -n '^.{80,}$'"

(sorry for all the questions, I have no machine to test this on)

robert914 commented 7 years ago

No problem, glad to help!

That one runs, but returns nothing:

$ bash -c "/usr/bin/find src -regex '.*\\.clj[scx]?' | xargs egrep -H -n '^.{80,}$'"
$ 
robert914 commented 7 years ago

If you change the "?" to a "*" it will work:

$ bash -c "/usr/bin/find src -regex '.*\\.clj[scx]*' | xargs egrep -H -n '^.{80,}$'"
src/hellworld/core.clj:7:  (println "This line is too long and should fail 80 character limit but no longer does in 0.4.0 of Bikeshed")
robert914 commented 7 years ago

Or, if you use the "-E" to use extended regex in BSD find:

$ bash -c "/usr/bin/find -E src -regex '.*\\.clj[scx]?' | xargs egrep -H -n '^.{80,}$'"
src/hellworld/core.clj:7:  (println "This line is too long and should fail 80 character limit but no longer does in 0.4.0 of Bikeshed")
dakrone commented 7 years ago

Or, if you use the "-E" to use extended regex in BSD find:

Unfortunately gnu find doesn't support -E, I'll push a commit to fix this using the * and maybe in the future we can revisit doing it without find (hey then it'd work on Windows :P)

robert914 commented 7 years ago

Sure, I wasn't sure if you had OS detection in the code to handle the differences

dakrone commented 7 years ago

Sure, I wasn't sure if you had OS detection in the code to handle the differences

Not yet!

dakrone commented 7 years ago

@robert914 I've released 0.4.1, can you give it a shot and make sure it fixes the problem?

robert914 commented 7 years ago

Yes, I confirm. It fixes the problem in the sample project I sent, as well as my larger project where I hit the problem originally.

Thanks for the quick fix!!

dakrone commented 7 years ago

@robert914 thanks for reporting it! I would never have known otherwise!