UnusedLetBind: speed up

We only need to check each piece of code for free variables once

src/Nix/Linter/Checks.hs

@@ -25,8 +25,9 @@ import           Nix.Expr.Types
 import           Nix.Expr.Types.Annotated
 
 import           Nix.Linter.Tools
+import           Nix.Linter.Tools.FreeVars
 import           Nix.Linter.Types
-import           Nix.Linter.Utils         (choose, sorted, (<$$>), (<&>))
+import           Nix.Linter.Utils         (choose, sorted, (<$$>))
 
 varName :: Text
 varName = "varName"
@@ -35,15 +36,16 @@ checkUnusedLetBinding :: CheckBase
 checkUnusedLetBinding warn e = [ (warn UnusedLetBind)
   & setLoc loc
   & note' varName name
-  | NLet_ srcSpan binds usedIn <- [unFix e]
-  , (bind, others) <- choose binds
+  | NLet_ _ binds _ <- [unFix e]
+  , free <- [freeVarsIgnoreTopBinds' e]
+  , bind <- binds
   , (name, loc) <- case bind of
       NamedVar (StaticKey name :| []) _ loc -> [(name, loc)]
       Inherit _ keys                    loc -> do
         StaticKey name <- keys
         [(name, loc)]
       _ -> []
-  , noRef name $ Fix $ NLet_ srcSpan others usedIn
+  , not $ Set.member name free
   ]
 
 checkUnusedArg :: CheckBase
@@ -70,10 +72,18 @@ checkEmptyInherit warn e = [ (warn EmptyInherit) {pos=singletonSpan loc}
 checkUnneededRec :: CheckBase
 checkUnneededRec warn e = [ warn UnneededRec
   | NSet_ _ann NRecursive binds <- [unFix e]
-  , not $ or $ choose binds <&> \case
-    (bind, others) -> case bind of
-      NamedVar (StaticKey name :| []) _ _ -> all (noRef name) (values others)
-      _                                   -> False
+  , not . or $ do
+    (bind, others) <- choose binds
+    name <- case bind of
+      NamedVar (StaticKey name :| []) _ _ -> [name]
+      Inherit _ keys                    _ -> do
+        StaticKey name <- keys
+        [name]
+      -- References to dynamic keys are not allowed
+      -- e.g. rec { x = "hi"; ${x} = 5; y = hi; }
+      -- will fail to evaluate, so we don't need to account for it
+      _ -> []
+    pure $ hasRef name $ Fix $ NSet_ _ann NRecursive others
   ]
 
 checkOpBase :: OffenseCategory -> Pair (UnwrappedNExprLoc -> Bool) -> NBinaryOp -> Bool -> CheckBase

src/Nix/Linter/Tools/FreeVars.hs

@@ -1,9 +1,48 @@
-module Nix.Linter.Tools.FreeVars (freeVars, freeVars') where
+module Nix.Linter.Tools.FreeVars (freeVars, freeVars', freeVarsIgnoreTopBinds
+                                 , freeVarsIgnoreTopBinds') where
 import           Data.Set                 (Set)
 
 import           Nix.Expr.Types
 import           Nix.Expr.Types.Annotated
 import           Nix.TH                   (freeVars)
 
+import qualified Data.Set                 as Set
+import           Data.Maybe               (mapMaybe)
+import           Data.Fix                 (unFix, Fix (..))
+
 freeVars' :: NExprLoc -> Set VarName
 freeVars' = freeVars . stripAnnotation
+
+freeVarsIgnoreTopBinds' :: NExprLoc -> Set VarName
+freeVarsIgnoreTopBinds' = freeVarsIgnoreTopBinds . stripAnnotation
+
+-- gets the free variables of the expression, assuming the top level bindings
+-- were not bound. Note that this function is *not* recursive, since we want to
+-- count bindings deeper in the tree
+freeVarsIgnoreTopBinds :: NExpr -> Set VarName
+freeVarsIgnoreTopBinds e =
+  case unFix e of
+    (NSet NRecursive bindings) -> bindFreeVars bindings
+    (NAbs (Param _) expr) -> freeVars expr
+    (NAbs (ParamSet set _ _) expr) ->
+      freeVars expr <> (Set.unions $ freeVars <$> mapMaybe snd set)
+    (NLet bindings expr) -> freeVars expr <> bindFreeVars bindings
+    noTopBinds -> freeVars $ Fix noTopBinds
+ where
+  bindFreeVars :: Foldable t => t (Binding NExpr) -> Set VarName
+  bindFreeVars = foldMap bind1Free
+   where
+    bind1Free :: Binding NExpr -> Set VarName
+    bind1Free (Inherit  Nothing     keys _) = Set.fromList $ mapMaybe staticKey keys
+    bind1Free (Inherit (Just scope) _    _) = freeVars scope
+    bind1Free (NamedVar path        expr _) = pathFree path <> freeVars expr
+
+  staticKey :: NKeyName r -> Maybe VarName
+  staticKey (StaticKey  varname) = pure varname
+  staticKey (DynamicKey _      ) = mempty
+
+  pathFree :: NAttrPath NExpr -> Set VarName
+  pathFree = foldMap mapFreeVars
+
+  mapFreeVars :: Foldable t => t NExpr -> Set VarName
+  mapFreeVars = foldMap freeVars