ndmitchell / record-dot-preprocessor

A preprocessor for a Haskell record syntax using dot
Other
129 stars 19 forks source link

Refactoring warnings reported by hlint #37

Closed dgellow closed 3 years ago

dgellow commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

@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 commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

@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 commented 3 years ago

@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 commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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

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

dgellow commented 3 years ago

Quite a lot of things as far as I understand: https://github.com/haskell/haskell-language-server/issues/297

ndmitchell commented 3 years ago

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.