Skip to content

Commit 1b187fd

Browse files
authored
Merge pull request #11825 from cabalism/hlint/suggest-nub-alternatives
Hlint: suggest nub alternative
2 parents 03d6011 + 64d8543 commit 1b187fd

25 files changed

Lines changed: 86 additions & 50 deletions

File tree

.hlint.yaml

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,10 @@
3737
- ignore: {name: "Use fmap", within: [Distribution.Client.HttpUtils, Distribution.Simple.SrcDist]}
3838
- ignore: {name: "Use fold", within: [Test.Laws]}
3939

40+
# For lists in these rules we use single letter variables like x rather than xs,
41+
# because "hlint considers only single-letter identifiers as rules' variables.
42+
# Anything longer will match literally".
43+
# SEE: https://github.com/ndmitchell/hlint/issues/1612
4044
- group:
4145
name: cabal-suggestions
4246
enabled: true
@@ -45,6 +49,14 @@
4549
lhs: fromMaybe x (Data.Map.lookup k m)
4650
rhs: Map.findWithDefault x k m
4751
name: Use findWithDefault
52+
- hint:
53+
lhs: nub x
54+
rhs: ordNub x
55+
name: Use ordNub
56+
note: |
57+
The growth rate of nub is O(n^2) and for ordNub it is O(n log n). May require adding import of either;
58+
- Distribution.Utils.Generic from Cabal-syntax, or
59+
- Distribution.Simple.Utils from Cabal.
4860
4961
- arguments:
5062
- --ignore-glob=Cabal-syntax/src/Distribution/Fields/Lexer.hs

Cabal-syntax/src/Distribution/PackageDescription/FieldGrammar.hs

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -88,6 +88,7 @@ import qualified Data.ByteString.Char8 as BS8
8888
import qualified Distribution.Compat.CharParsing as P
8989
import qualified Distribution.SPDX as SPDX
9090
import qualified Distribution.Types.Lens as L
91+
import Distribution.Utils.Generic (ordNub)
9192

9293
-------------------------------------------------------------------------------
9394
-- PackageDescription
@@ -901,7 +902,7 @@ _syntaxFieldNames =
901902
sequence_
902903
[ BS8.putStrLn $ " \\ " <> n
903904
| n <-
904-
nub $
905+
ordNub $
905906
sort $
906907
mconcat
907908
[ fieldGrammarKnownFieldList packageDescriptionFieldGrammar
@@ -927,7 +928,7 @@ _syntaxExtensions =
927928
]
928929
where
929930
es =
930-
nub $
931+
ordNub $
931932
sort
932933
[ prettyShow e
933934
| e <- [minBound .. maxBound]

Cabal-syntax/src/Distribution/Types/BuildInfo.hs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@ import Distribution.Utils.Path
2828

2929
import Distribution.Compiler
3030
import Distribution.ModuleName
31+
import Distribution.Utils.Generic (ordNub)
3132
import Language.Haskell.Extension
3233

3334
-- Consider refactoring into executable and library versions.
@@ -260,7 +261,7 @@ instance Semigroup BuildInfo where
260261
}
261262
where
262263
combine field = field a `mappend` field b
263-
combineNub field = nub (combine field)
264+
combineNub field = ordNub (combine field)
264265
combineMby field = field b `mplus` field a
265266

266267
emptyBuildInfo :: BuildInfo

Cabal-tests/tests/UnitTests/Distribution/PackageDescription/Check.hs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import Distribution.Compat.Prelude
77
import Prelude ()
88

99
import Distribution.PackageDescription.Check
10+
import Distribution.Simple.Utils (ordNub)
1011

1112
import Test.Tasty
1213
import Test.Tasty.HUnit
@@ -26,7 +27,7 @@ tests =
2627
allExplanationIdStrings = map ppCheckExplanationId [minBound..maxBound]
2728

2829
uniqueNames :: Bool
29-
uniqueNames = length allExplanationIdStrings == length (nub allExplanationIdStrings)
30+
uniqueNames = length allExplanationIdStrings == length (ordNub allExplanationIdStrings)
3031

3132
longerThan :: [CheckExplanationIDString]
3233
longerThan = filter ((>25). length) allExplanationIdStrings

Cabal-tests/tests/UnitTests/Distribution/Utils/NubList.hs

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import Prelude ()
88
import Distribution.Compat.Prelude.Internal
99

1010
import Distribution.Utils.NubList
11+
import Distribution.Simple.Utils (ordNub)
1112
import Test.Tasty
1213
import Test.Tasty.HUnit
1314
import Test.Tasty.QuickCheck
@@ -46,25 +47,25 @@ prop_Ordering :: [Int] -> Property
4647
prop_Ordering xs =
4748
mempty <> toNubList xs' === toNubList xs' <> mempty
4849
where
49-
xs' = nub xs
50+
xs' = ordNub xs
5051

5152
prop_DeDupe :: [Int] -> Property
5253
prop_DeDupe xs =
5354
fromNubList (toNubList (xs' ++ xs)) === xs' -- Note, we append primeless xs
5455
where
55-
xs' = nub xs
56+
xs' = ordNub xs
5657

5758
prop_DeDupeR :: [Int] -> Property
5859
prop_DeDupeR xs =
5960
fromNubListR (toNubListR (xs ++ xs')) === xs' -- Note, we prepend primeless xs
6061
where
61-
xs' = nub xs
62+
xs' = ordNub xs
6263

6364
prop_Nub :: [Int] -> Property
6465
prop_Nub xs = rhs === lhs
6566
where
6667
rhs = fromNubList (toNubList xs)
67-
lhs = nub xs
68+
lhs = ordNub xs
6869

6970
prop_Identity :: [Int] -> Bool
7071
prop_Identity xs =

Cabal/src/Distribution/PackageDescription/Check/Common.hs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ import Distribution.Compat.NonEmptySet (toNonEmpty)
2626
import Distribution.Package
2727
import Distribution.PackageDescription
2828
import Distribution.PackageDescription.Check.Monad
29+
import Distribution.Simple.Utils (ordNub)
2930
import Distribution.Utils.Generic (isAscii)
3031
import Distribution.Version
3132

@@ -80,7 +81,7 @@ partitionDeps ads ns ds = do
8081
-- shared targets that match
8182
fads = filter (flip elem dqs . fst) ads
8283
-- the names of such targets
83-
inName = nub $ map fst fads :: [UnqualComponentName]
84+
inName = ordNub $ map fst fads :: [UnqualComponentName]
8485
-- the dependencies of such targets
8586
inDep = concatMap snd fads :: [Dependency]
8687

Cabal/src/Distribution/PackageDescription/Check/Target.hs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -559,8 +559,8 @@ checkBuildInfoFeatures bi sv = do
559559
checkBuildInfoExtensions :: Monad m => BuildInfo -> CheckM m ()
560560
checkBuildInfoExtensions bi = do
561561
let exts = allExtensions bi
562-
extCabal1_2 = nub $ filter (`elem` compatExtensionsExtra) exts
563-
extCabal1_4 = nub $ filter (`notElem` compatExtensions) exts
562+
extCabal1_2 = ordNub $ filter (`elem` compatExtensionsExtra) exts
563+
extCabal1_4 = ordNub $ filter (`notElem` compatExtensions) exts
564564
-- As of Cabal-1.4 we can add new extensions without worrying
565565
-- about breaking old versions of cabal.
566566
checkSpecVer

Cabal/src/Distribution/Simple.hs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -824,8 +824,8 @@ sanityCheckHookedBuildInfo verbosity pkg_descr (_, hookExes)
824824
| exe1 : _ <- nonExistent =
825825
dieWithException verbosity $ SanityCheckHookedBuildInfo exe1
826826
where
827-
pkgExeNames = nub (map exeName (executables pkg_descr))
828-
hookExeNames = nub (map fst hookExes)
827+
pkgExeNames = ordNub (map exeName (executables pkg_descr))
828+
hookExeNames = ordNub (map fst hookExes)
829829
nonExistent = hookExeNames \\ pkgExeNames
830830
sanityCheckHookedBuildInfo _ _ _ = return ()
831831

Cabal/src/Distribution/Simple/BuildTarget.hs

Lines changed: 14 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
{-# LANGUAGE DuplicateRecordFields #-}
44
{-# LANGUAGE FlexibleContexts #-}
55
{-# LANGUAGE RankNTypes #-}
6+
{-# LANGUAGE StandaloneDeriving #-}
67
{-# LANGUAGE TupleSections #-}
78

89
-----------------------------------------------------------------------------
@@ -132,6 +133,9 @@ data BuildTarget
132133
BuildTargetFile ComponentName FilePath
133134
deriving (Eq, Show, Generic)
134135

136+
-- | @since 3.18
137+
deriving instance Ord BuildTarget
138+
135139
instance Binary BuildTarget
136140

137141
buildTargetComponentName :: BuildTarget -> ComponentName
@@ -855,6 +859,9 @@ data MatchError
855859
| MatchErrorNoSuch String String
856860
deriving (Show, Eq)
857861

862+
-- | @since 3.18
863+
deriving instance Ord MatchError
864+
858865
instance Alternative Match where
859866
empty = mzero
860867
(<|>) = mplus
@@ -943,13 +950,13 @@ increaseConfidence = ExactMatch 1 [()]
943950
increaseConfidenceFor :: Match a -> Match a
944951
increaseConfidenceFor m = m >>= \r -> increaseConfidence >> return r
945952

946-
nubMatches :: Eq a => Match a -> Match a
953+
nubMatches :: Ord a => Match a -> Match a
947954
nubMatches (NoMatch d msgs) = NoMatch d msgs
948-
nubMatches (ExactMatch d xs) = ExactMatch d (nub xs)
949-
nubMatches (InexactMatch d xs) = InexactMatch d (nub xs)
955+
nubMatches (ExactMatch d xs) = ExactMatch d (ordNub xs)
956+
nubMatches (InexactMatch d xs) = InexactMatch d (ordNub xs)
950957

951958
nubMatchErrors :: Match a -> Match a
952-
nubMatchErrors (NoMatch d msgs) = NoMatch d (nub msgs)
959+
nubMatchErrors (NoMatch d msgs) = NoMatch d (ordNub msgs)
953960
nubMatchErrors (ExactMatch d xs) = ExactMatch d xs
954961
nubMatchErrors (InexactMatch d xs) = InexactMatch d xs
955962

@@ -970,14 +977,14 @@ tryEach = exactMatches
970977
-- | Given a matcher and a key to look up, use the matcher to find all the
971978
-- possible matches. There may be 'None', a single 'Unambiguous' match or
972979
-- you may have an 'Ambiguous' match with several possibilities.
973-
findMatch :: Eq b => Match b -> MaybeAmbiguous b
980+
findMatch :: Ord b => Match b -> MaybeAmbiguous b
974981
findMatch match =
975982
case match of
976-
NoMatch _ msgs -> None (nub msgs)
983+
NoMatch _ msgs -> None (ordNub msgs)
977984
ExactMatch _ xs -> checkAmbiguous xs
978985
InexactMatch _ xs -> checkAmbiguous xs
979986
where
980-
checkAmbiguous xs = case nub xs of
987+
checkAmbiguous xs = case ordNub xs of
981988
[x] -> Unambiguous x
982989
xs' -> Ambiguous xs'
983990

Cabal/src/Distribution/Simple/Configure.hs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1369,14 +1369,14 @@ finalCheckPackage
13691369
-- Check languages and extensions
13701370
-- TODO: Move this into a helper function.
13711371
let langlist =
1372-
nub $
1372+
ordNub $
13731373
mapMaybe defaultLanguage (enabledBuildInfos pkg_descr enabled)
13741374
let langs = unsupportedLanguages comp langlist
13751375
unless (null langs) $
13761376
dieWithException verbosity $
13771377
UnsupportedLanguages (packageId pkg_descr) (compilerId comp) (map prettyShow langs)
13781378
let extlist =
1379-
nub $
1379+
ordNub $
13801380
concatMap
13811381
allExtensions
13821382
(enabledBuildInfos pkg_descr enabled)
@@ -2607,7 +2607,7 @@ configurePkgconfigPackages verbosity pkg_descr progdb enabled
26072607
pkgconfigBuildInfo :: [PkgconfigDependency] -> IO BuildInfo
26082608
pkgconfigBuildInfo [] = return mempty
26092609
pkgconfigBuildInfo pkgdeps = do
2610-
let pkgs = nub [prettyShow pkg | PkgconfigDependency pkg _ <- pkgdeps]
2610+
let pkgs = ordNub [prettyShow pkg | PkgconfigDependency pkg _ <- pkgdeps]
26112611
ccflags <- pkgconfig ("--cflags" : pkgs)
26122612
ldflags <- pkgconfig ("--libs" : pkgs)
26132613
ldflags_static <- pkgconfig ("--libs" : "--static" : pkgs)

0 commit comments

Comments
 (0)