HiveBrain v1.2.0
Get Started
← Back to all entries
patternjavaMinor

Haskell tool for find @SuppressWarnings in Java source code

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
suppresswarningssourcejavaforhaskellfindcodetool

Problem

I put together a little tool for finding where people have suppressed warnings, and to see if they have commented why. Its in Haskell for fun and some practice. Its split into two files, the thinking here was to keep all of the IO functions in one place and the pure parser code in another.
My immediate concerns are:

  • I use Text, Text.Lazy fairly indiscriminately, not sure if there is a better way?



  • It works blazingly fast on a pure source code tree but on an entire project tree, with binaries in it, it seems to stall, so I think either I'm not guarding against reading the binaries or there is a space leak?



Main.hs

```
{-# LANGUAGE DeriveDataTypeable #-}

module Main where

import Control.Arrow
import qualified Data.Foldable as F
import qualified Data.Text as T
import qualified Data.Text.Lazy as TL
import qualified Data.Text.IO as TIO
import qualified Data.Text.Lazy.IO as TLIO
import JavaParser
import System.Directory.Tree
import System.Environment
import System.FilePath
import Text.Hastache
import Text.Hastache.Context
import Data.Data
import Data.Generics
import Data.Functor.Identity

main :: IO ()
main = do
args IO ()
handleArgs [templatePath, outputPath, sourceDir] = walkFiles templatePath outputPath sourceDir
handleArgs _ = putStrLn "Usage: suppr "

-- | the real main entry point after handling CL args
walkFiles :: FilePath -> FilePath -> FilePath -> IO ()
walkFiles templatePath outputPath sourceDir = do
tree Tables -> IO TL.Text
render template = hastacheStr defaultConfig (encodeStr template) . mkGenericContext

-- | Given a source file build the actual tables of unjust and just suppressions
processFile :: (FilePath, T.Text) -> Tables -> Tables
processFile (source, content) (Tables unjust just) =
makeTables Tables
makeTables (unjust, just) = Tables unjust just

-- | Add new table to list of tables i.e. add the lines for a single file
addTable :: FilePath -> [Table] -> [Line] -> [Table]
addTable source table [] = table
addTable source table lin

Solution

-
haskell is Lazy, so should you be. You should consider using readDirectoryWithL over readDirectoryWith. That should have no heavy performance implications but still allow us to save a lot of memory.

-
testTemplate is a bit confusing as a name. Why not outputTemplate? This IMO fits the usage better.

-
You're not quite consistent as to when you use import qualified. I'd have expected Data.Attoparsec.Text to be imported qualified.

-
UX for erroneous arguments is bad. I'm not told that outputPath and templatePath better be files instead of directories. I'm not quite sure as to how TLIO handles this, but I expect problems in case this is used incorrectly.

-
Nitpick + Design Critique: I'd prefer a multiline comment when explaining what Line is / does. You could even go so far as to not force the grouping into Justified and Unjustified into the types you define and instead use the following Line:

data Line = Suppression { rule :: T.Text, reason :: Maybe T.Text }
          | Code deriving (Show, Eq, Data, Typeable)


Then again this seems like bad FP advice from a mostly OO person, so ... Note that it would make isSuppression a bit more straightforward and greatly simplify suppressionParser and functions called by it. It also makes sure you're not going to have that an easy time partitioning that list.

The partition (not.isJustified) shouldn't be the parser's responsibility. getSuppressions should just return a [Line] consuming the results is the caller's responsibility.

-
The Data.Attoparsec.ByteString documentation explicitly states to use takeWhile1 over many1 where possible. To be more correct it encourages using the ByteString parsers over the others. In the current case I'm reasonably sure you can rework your code to use scan and consume the lines you're looking for.

Code Snippets

data Line = Suppression { rule :: T.Text, reason :: Maybe T.Text }
          | Code deriving (Show, Eq, Data, Typeable)

Context

StackExchange Code Review Q#140307, answer score: 3

Revisions (0)

No revisions yet.