haddock icon indicating copy to clipboard operation
haddock copied to clipboard

Linting in the codebase

Open philderbeast opened this issue 3 years ago • 3 comments

about the usage of HLint in the codebase. I am not against it, but I would rather have a fixed set of lints to follow before reviewing linting PRs.

Originally posted by @Kleidukos in https://github.com/haskell/haddock/issues/1335#issuecomment-782268765

I have a small refinement for hlint (not yet merged) that shows hint occurrences. Thought this might be useful for discussing hlint of haddock. Note that I had to comment out 3 lines of CPP #ifdef to get hlint to follow through.

in ./haddock-api/src/Haddock/InterfaceFile.hs  at line 103 col 1
CallStack (from HasCallStack):
  error, called at ./Language/Preprocessor/Cpphs/CppIfdef.hs:113:21 in
  cpphs-1.20.9.1-0ade0003:Language.Preprocessor.Cpphs.CppIfdef
...
Warning: Can't find file "HsVersions.h" in directories
	./haddock-api/src/Haddock
	.
  Asked for by: ./haddock-api/src/Haddock/Convert.hs  at line 22 col 1
hlint: # error This module doesn't provide compat-shims for versions prior to base-4.5
in ./haddock-library/src/CompatPrelude.hs  at line 4 col 1
CallStack (from HasCallStack):
  error, called at ./Language/Preprocessor/Cpphs/CppIfdef.hs:113:21 in
  cpphs-1.20.9.1-0ade0003:Language.Preprocessor.Cpphs.CppIfdef
...
hlint: # error This module doesn't provide compat-shims for versions prior to base-4.5
in ./haddock-library/src/CompatPrelude.hs  at line 4 col 1
CallStack (from HasCallStack):
  error, called at ./Language/Preprocessor/Cpphs/CppIfdef.hs:113:21 in
  cpphs-1.20.9.1-0ade0003:Language.Preprocessor.Cpphs.CppIfdef
haddock on master > hlint --willful-ignorance . 
# Warnings currently triggered by your code
- ignore: {name: "Avoid lambda"} # 3 hints
- ignore: {name: "Avoid lambda using `infix`"} # 5 hints
- ignore: {name: "Eta reduce"} # 18 hints
- ignore: {name: "Functor law"} # 3 hints
- ignore: {name: "Fuse concatMap/map"} # 1 hint
- ignore: {name: "Fuse foldr/map"} # 2 hints
- ignore: {name: "Fuse mapM/map"} # 2 hints
- ignore: {name: "Hoist not"} # 1 hint
- ignore: {name: "Move brackets to avoid $"} # 7 hints
- ignore: {name: "Move guards forward"} # 1 hint
- ignore: {name: "Move map inside list comprehension"} # 2 hints
- ignore: {name: "Redundant $"} # 11 hints
- ignore: {name: "Redundant <$>"} # 3 hints
- ignore: {name: "Redundant bang pattern"} # 1 hint
- ignore: {name: "Redundant bracket"} # 44 hints
- ignore: {name: "Redundant id"} # 1 hint
- ignore: {name: "Redundant if"} # 3 hints
- ignore: {name: "Redundant lambda"} # 2 hints
- ignore: {name: "Redundant map"} # 3 hints
- ignore: {name: "Redundant return"} # 1 hint
- ignore: {name: "Replace case with fromMaybe"} # 2 hints
- ignore: {name: "Replace case with maybe"} # 4 hints
- ignore: {name: "Unused LANGUAGE pragma"} # 21 hints
- ignore: {name: "Use $>"} # 4 hints
- ignore: {name: "Use ++"} # 3 hints
- ignore: {name: "Use :"} # 3 hints
- ignore: {name: "Use <$>"} # 12 hints
- ignore: {name: "Use <&>"} # 1 hint
- ignore: {name: "Use <|>"} # 1 hint
- ignore: {name: "Use =<<"} # 1 hint
- ignore: {name: "Use all"} # 1 hint
- ignore: {name: "Use bimap"} # 1 hint
- ignore: {name: "Use camelCase"} # 31 hints
- ignore: {name: "Use concatMap"} # 1 hint
- ignore: {name: "Use const"} # 4 hints
- ignore: {name: "Use fewer LANGUAGE pragmas"} # 1 hint
- ignore: {name: "Use fewer imports"} # 1 hint
- ignore: {name: "Use find"} # 1 hint
- ignore: {name: "Use fmap"} # 12 hints
- ignore: {name: "Use foldr"} # 1 hint
- ignore: {name: "Use fromMaybe"} # 2 hints
- ignore: {name: "Use infix"} # 2 hints
- ignore: {name: "Use intercalate"} # 1 hint
- ignore: {name: "Use isAsciiLower"} # 1 hint
- ignore: {name: "Use isAsciiUpper"} # 1 hint
- ignore: {name: "Use isDigit"} # 1 hint
- ignore: {name: "Use lambda-case"} # 4 hints
- ignore: {name: "Use let"} # 2 hints
- ignore: {name: "Use list comprehension"} # 2 hints
- ignore: {name: "Use list literal pattern"} # 1 hint
- ignore: {name: "Use map once"} # 1 hint
- ignore: {name: "Use maybe"} # 2 hints
- ignore: {name: "Use newtype instead of data"} # 13 hints
- ignore: {name: "Use notElem"} # 2 hints
- ignore: {name: "Use print"} # 3 hints
- ignore: {name: "Use record patterns"} # 5 hints
- ignore: {name: "Use sortOn"} # 3 hints
- ignore: {name: "Use tuple-section"} # 2 hints
- ignore: {name: "Use unless"} # 1 hint
- ignore: {name: "Use void"} # 1 hint

philderbeast avatar Apr 26 '22 10:04 philderbeast

This is excellent, thank you very much for this!

Kleidukos avatar Apr 26 '22 10:04 Kleidukos

Clearly the unused language pragma are the most pressing stuff to remove from the codebase. :) Redundant bang patterns as well.

Kleidukos avatar Apr 26 '22 10:04 Kleidukos

@Kleidukos once we have the hlint action passing, we could then chip away at the hints as we please, starting as you suggest with:

  - ignore: {name: "Avoid lambda"} # 3 hints
  - ignore: {name: "Avoid lambda using `infix`"} # 5 hints
  - ignore: {name: "Eta reduce"} # 18 hints
  - ignore: {name: "Functor law"} # 3 hints
  - ignore: {name: "Fuse concatMap/map"} # 1 hint
  - ignore: {name: "Fuse foldr/map"} # 2 hints
  - ignore: {name: "Fuse mapM/map"} # 2 hints
  - ignore: {name: "Hoist not"} # 1 hint
  - ignore: {name: "Move brackets to avoid $"} # 7 hints
  - ignore: {name: "Move guards forward"} # 1 hint
  - ignore: {name: "Move map inside list comprehension"} # 2 hints
  - ignore: {name: "Redundant $"} # 11 hints
  - ignore: {name: "Redundant <$>"} # 3 hints
- - ignore: {name: "Redundant bang pattern"} # 1 hint
  - ignore: {name: "Redundant bracket"} # 44 hints
  - ignore: {name: "Redundant id"} # 1 hint
  - ignore: {name: "Redundant if"} # 3 hints
  - ignore: {name: "Redundant lambda"} # 2 hints
  - ignore: {name: "Redundant map"} # 3 hints
  - ignore: {name: "Redundant return"} # 1 hint
  - ignore: {name: "Replace case with fromMaybe"} # 2 hints
  - ignore: {name: "Replace case with maybe"} # 4 hints
- - ignore: {name: "Unused LANGUAGE pragma"} # 21 hints
  - ignore: {name: "Use $>"} # 4 hints
  - ignore: {name: "Use ++"} # 3 hints
  - ignore: {name: "Use :"} # 3 hints
  - ignore: {name: "Use <$>"} # 12 hints
  - ignore: {name: "Use <&>"} # 1 hint
  - ignore: {name: "Use <|>"} # 1 hint
  - ignore: {name: "Use =<<"} # 1 hint
  - ignore: {name: "Use all"} # 1 hint
  - ignore: {name: "Use bimap"} # 1 hint
  - ignore: {name: "Use camelCase"} # 31 hints
  - ignore: {name: "Use concatMap"} # 1 hint
  - ignore: {name: "Use const"} # 4 hints
  - ignore: {name: "Use fewer LANGUAGE pragmas"} # 1 hint
  - ignore: {name: "Use fewer imports"} # 1 hint
  - ignore: {name: "Use find"} # 1 hint
  - ignore: {name: "Use fmap"} # 12 hints
  - ignore: {name: "Use foldr"} # 1 hint
  - ignore: {name: "Use fromMaybe"} # 2 hints
  - ignore: {name: "Use infix"} # 2 hints
  - ignore: {name: "Use intercalate"} # 1 hint
  - ignore: {name: "Use isAsciiLower"} # 1 hint
  - ignore: {name: "Use isAsciiUpper"} # 1 hint
  - ignore: {name: "Use isDigit"} # 1 hint
  - ignore: {name: "Use lambda-case"} # 4 hints
  - ignore: {name: "Use let"} # 2 hints
  - ignore: {name: "Use list comprehension"} # 2 hints
  - ignore: {name: "Use list literal pattern"} # 1 hint
  - ignore: {name: "Use map once"} # 1 hint
  - ignore: {name: "Use maybe"} # 2 hints
  - ignore: {name: "Use newtype instead of data"} # 13 hints
  - ignore: {name: "Use notElem"} # 2 hints
  - ignore: {name: "Use print"} # 3 hints
  - ignore: {name: "Use record patterns"} # 5 hints
  - ignore: {name: "Use sortOn"} # 3 hints
  - ignore: {name: "Use tuple-section"} # 2 hints
  - ignore: {name: "Use unless"} # 1 hint
  - ignore: {name: "Use void"} # 1 hint

philderbeast avatar Apr 26 '22 15:04 philderbeast