Skip to content

Refactoring warnings reported by hlint #37

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Closed
dgellow opened this issue Feb 13, 2021 · 12 comments
Closed

Refactoring warnings reported by hlint #37

dgellow opened this issue Feb 13, 2021 · 12 comments

Comments

@dgellow
Copy link

dgellow commented Feb 13, 2021

Hi there,

while experimenting with this preprocessor I get a lot of refactoring suggestions from hlint. Is there a way to disable hlint warnings for the code generated by the preprocessor, or alternatively would it be possible to automatically wrap the generated code in a ifndef __HLINT__, or automatically add a pragma to ignore the refactoring suggestions? Any solution that doesn't require to disable the whole category of warning in hlint would be appreciated.

Example of generated code

File src/SometingGenerated.hs, output of record-dot-preprocessor .\src\Something.hs > .\src\SomethingGenerated.hs .

{-# LINE 1 ".\\src\\Something.hs" #-}
{-# LANGUAGE DuplicateRecordFields, DataKinds, FlexibleInstances, TypeApplications, FlexibleContexts, MultiParamTypeClasses, OverloadedLabels, TypeFamilies, TypeOperators, GADTs, UndecidableInstances #-}
{-# OPTIONS_GHC -F -pgmF=record-dot-preprocessor #-}
{-# LINE 2 ".\\src\\Something.hs" #-}

module Something where


import qualified GHC.Records.Extra as Z
data Person = Person {name :: String, age :: Int}
{-# LINE 6 ".\\src\\Something.hs" #-}

data Product = Product {name :: String, id :: String}

instance (aplg ~ (String)) => Z.HasField "name" (Person) aplg where hasField _r = (\_x -> case _r of {(Person _ _x2) -> Person _x _x2}, case _r of {(Person _x1 _) -> _x1})
instance (aplg ~ (Int)) => Z.HasField "age" (Person) aplg where hasField _r = (\_x -> case _r of {(Person _x1 _) -> Person _x1 _x}, case _r of {(Person _ _x1) -> _x1})
instance (aplg ~ (String)) => Z.HasField "name" (Product) aplg where hasField _r = (\_x -> case _r of {(Product _ _x2) -> Product _x _x2}, case _r of {(Product _x1 _) -> _x1})
instance (aplg ~ (String)) => Z.HasField "id" (Product) aplg where hasField _r = (\_x -> case _r of {(Product _x1 _) -> Product _x1 _x}, case _r of {(Product _ _x1) -> _x1})
_preprocessor_unused_Something :: Z.HasField "" r a => r -> a;_preprocessor_unused_Something = Z.getField @""

Reported hlint issues

$ hlint .\src\SomethingGenerated.hs
src\Something.hs:9:18-25: Warning: Redundant bracket
Found:
  (String)
Perhaps:
  String

src\Something.hs:9:49-56: Warning: Redundant bracket
Found:
  (Person)
Perhaps:
  Person

src\Something.hs:10:18-22: Warning: Redundant bracket
Found:
  (Int)
Perhaps:
  Int

src\Something.hs:10:45-52: Warning: Redundant bracket
Found:
  (Person)
Perhaps:
  Person

src\Something.hs:11:18-25: Warning: Redundant bracket
Found:
  (String)
Perhaps:
  String

src\Something.hs:11:49-57: Warning: Redundant bracket
Found:
  (Product)
Perhaps:
  Product

src\Something.hs:12:18-25: Warning: Redundant bracket
Found:
  (String)
Perhaps:
  String

src\Something.hs:12:47-55: Warning: Redundant bracket
Found:
  (Product)
Perhaps:
  Product

src\Something.hs:13:1-61: Suggestion: Use camelCase
Found:
  _preprocessor_unused_Something :: Z.HasField "" r a => r -> a
Perhaps:
  _preprocessorUnusedSomething :: Z.HasField "" r a => r -> a

src\Something.hs:13:63-109: Suggestion: Use camelCase
Found:
  _preprocessor_unused_Something = ...
Perhaps:
  _preprocessorUnusedSomething = ...

11 hints
@ndmitchell
Copy link
Owner

Looking at that list of hints - there are two forms - where we wrap something in brackets that didn't need to be (a simplistic all isAlpha check looks like it would get rid of that) and the fact that the camel case suggestion is tripping (super easy to remove, and I'd argue HLint is being a big picky too). So it seems like of the options, removing the HLint suggestions by modifying the preprocessor might be the right thing to do.

Although why is the preprocessor output being run through HLint? Are you checking in the preprocessor output rather than running it through with the processor from GHC? Are you using this in HLS?

@dgellow
Copy link
Author

dgellow commented Feb 15, 2021

Although why is the preprocessor output being run through HLint? Are you checking in the preprocessor output rather than running it through with the processor from GHC? Are you using this in HLS?

I'm using the Haskell VSCode extension with no modification or customization, it is based on Haskell Language Server.

In my project I run the preprocessor by specifying the {-# OPTIONS_GHC -F -pgmF=record-dot-preprocessor #-} pragma and do not check in the preprocessor output, I added it to the issue description to make it clear where the problem comes from.

That looks like this in VSCode when working on my source files, as you can see it is very noisy:

image

Maybe disabling HLint for the preprocessor output might also be a solution? I'm not sure what are the benefits of linting the work of a preprocessor.

Edit: I'm super new to the Haskell world so I'm not sure of what I'm doing or if I'm reporting this issue to the correct project. Just tell me if it's instead an HSL problem, I can instead create an issue in their repository.

@georgefst
Copy link

@dgellow I've been hitting this as well, and the best thing I've found to reduce the noise is to deselect Show Infos from the filter menu in the VScode problems window. Given that the preprocessor is only a temporary workaround, and RecordDotSyntax is on the horizon, I can live with this.

@georgefst
Copy link

Note that running HLS on the unprocessed source won't work in general as it can't be parsed. Having HLS not run HLint at all on files where a preprocessor is active is an option, but seems like overkill.

ndmitchell added a commit that referenced this issue Feb 27, 2021
@ndmitchell
Copy link
Owner

I just released 0.2.9 of record-dot-preprocessor. There were two warnings coming out of the generated code that I could see - one about camel case (fixed) and one about excessive bracketing (I generate a HLINT pragma to ignore it). Now when running the generated code through HLint I get no warnings, so hopefully that fixes it for you.

@dgellow
Copy link
Author

dgellow commented Feb 28, 2021

@ndmitchell Thank you for doing this change. Unfortunately the generated code still has an underscore character that HLint doesn't like (it's really over-picky here...):

image

And for some reasons I don't fully get I still have refactoring hints for redundant bracket, even with the {- HLINT module ignore Redundant bracket -} pragma that you added.

My solution so far has been to add the following in all my source files:

{-# ANN module "HLint: ignore Use camelCase" #-}
{-# ANN module "HLint: ignore Redundant bracket" #-}

@dgellow
Copy link
Author

dgellow commented Feb 28, 2021

@georgefst:
the best thing I've found to reduce the noise is to deselect Show Infos from the filter menu in the VScode problems window.

That's an option but disabling all linter feedback is too much for my use.

ndmitchell added a commit that referenced this issue Mar 1, 2021
@ndmitchell
Copy link
Owner

I screwed up the HLint ignore pragma, so it wasn't having any effect. I've just pushed a new patch, are you able to try with a version built from HEAD? I'm not convinced it will fix the issue, but it then might point at a HLS/GHC bug. I think it should fix the camelCase issue though.

@dgellow
Copy link
Author

dgellow commented Mar 26, 2021

Hey @ndmitchell, I will eventually try out your changes. I'm just a bit stuck currently because of ... reasons. I had to reinstall my system, setup a haskell setup, realize that haskell-language-server doesn't work with GHC 9, figure out a way to rollback, fix cabal issues, and so on and so forth 🙃. Once I find the time to get everything up and running again I will give you some feedback. Sorry for taking so long!

@shayne-fletcher
Copy link
Collaborator

realize that haskell-language-server doesn't work with GHC 9

doesn't it? what's the issue there?

@dgellow
Copy link
Author

dgellow commented Mar 26, 2021

Quite a lot of things as far as I understand: haskell/haskell-language-server#297

@ndmitchell
Copy link
Owner

I released record-dot-preprocessor 0.2.10 since then, which I'm going to optimistically hope fixes the issue. I'll close this ticket, and take your time with trying this out (I'm well underwater with notifications etc myself!), but please reopen if it still occurs.

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants