bjpop / language-python

A parser for Python 2.x and 3.x written in Haskell
157 stars 45 forks source link

Warnings on build #46

Closed dperelman closed 5 years ago

dperelman commented 5 years ago

The build currently produces a handful a warnings. For example:

src/Language/Python/Common/ParseError.hs:25:10: warning: [-Wdeprecations]
    In the use of type constructor or class ‘Error’
    (imported from Control.Monad.Error.Class, but defined in Control.Monad.Trans.Error):
    Deprecated: "Use Control.Monad.Trans.Except instead"
   |                                 
25 | instance Error ParseError where 
   |          ^^^^^     
src/Language/Python/Common/ParserUtils.hs:98:1: warning: [-Wincomplete-patterns]
    Pattern match(es) are non-exhaustive
    In an equation for ‘makeTupleOrExpr’: Patterns not matched: [] _
   |                                 
98 | makeTupleOrExpr [e] Nothing = e 
   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^...

They mostly look pretty simple to fix; I've started a branch where I'm trying to do so. But I'm not super-familiar with Haskell style, so I'm not sure exactly how to properly fix those warnings or maybe if simply telling the compiler to not warn for those things is the proper fix.

(My impetus for doing so was that when I saw the warnings, I first thought they might mean the package was actually broken because it was taking a long time to compile. After a few minutes, it made progress, so I realized they were not actually breaking anything.)

bjpop commented 5 years ago

Thanks @dperelman

The warnings are useful in that they could point to bugs in the code.

It is useful to see if they can be individually fixed, but care must be made to make sure that whatever is done is in fact correcting the code and not introducing more bugs.

This library is known to cause trouble for GHC compile times; I suspect these warnings are not related to that problem.

dperelman commented 5 years ago

It looks like there's three categories of warnings, 2 simple, 1 more complicated:

  1. -Wincomplete-patterns ("Pattern match(es) are non-exhaustive"): These are options on patterns that never get used, so it doesn't matter that they're missing. I can add definitions if it makes sense or add error "This should be unreachable because $FOO.".
  2. -Wunused-imports (e.g., "The import of ‘isJust’ from module ‘Data.Maybe’ is redundant"): I assume these are just due to code changes without corresponding adjustments to the import statements. Should be straightforward to fix.
  3. -Wdeprecations ("In the use of type constructor or class ‘Error’ (imported from Control.Monad.Error.Class, but defined in Control.Monad.Trans.Error): Deprecated: "Use Control.Monad.Trans.Except instead""): This one looks more complicated. I fiddled around a little and wasn't able to come up with a quick fix... but it's also been a long time since I last touched Haskell, so I really didn't know what I was doing. Either someone who knows more about Haskell should take this on, or I'll spend more time on it in a separate PR.

I'll try to finish up looking at all the warnings in categories 1+2 and make a PR in the next couple days.

bjpop commented 5 years ago

Thanks! I agree, look at issues 1 and 2 first. There is a separate repository for testing the library: https://github.com/bjpop/language-python-test It is desirable to run those tests successfully before making any pull requests. It probably shouldn't be a separate repository, but it is there for historical reasons.