From b9f4bfe19815b3741cf455c2aa9238718da432ab Mon Sep 17 00:00:00 2001 From: Chris Penner Date: Fri, 5 Jul 2024 12:09:00 -0700 Subject: [PATCH 1/5] Add unused binding detection to LSP --- unison-cli/src/Unison/LSP/FileAnalysis.hs | 5 ++- .../Unison/LSP/FileAnalysis/UnusedBindings.hs | 32 +++++++++++++++++++ unison-cli/unison-cli.cabal | 1 + 3 files changed, 37 insertions(+), 1 deletion(-) create mode 100644 unison-cli/src/Unison/LSP/FileAnalysis/UnusedBindings.hs diff --git a/unison-cli/src/Unison/LSP/FileAnalysis.hs b/unison-cli/src/Unison/LSP/FileAnalysis.hs index f5f29b5e2..32a51af4a 100644 --- a/unison-cli/src/Unison/LSP/FileAnalysis.hs +++ b/unison-cli/src/Unison/LSP/FileAnalysis.hs @@ -35,6 +35,7 @@ import Unison.KindInference.Error qualified as KindInference import Unison.LSP.Conversions import Unison.LSP.Conversions qualified as Cv import Unison.LSP.Diagnostics (DiagnosticSeverity (..), mkDiagnostic, reportDiagnostics) +import Unison.LSP.FileAnalysis.UnusedBindings qualified as UnusedBindings import Unison.LSP.Orphans () import Unison.LSP.Types import Unison.LSP.VFS qualified as VFS @@ -111,12 +112,14 @@ checkFile doc = runMaybeT do & toRangeMap let typeSignatureHints = fromMaybe mempty (mkTypeSignatureHints <$> parsedFile <*> typecheckedFile) let fileSummary = FileSummary.mkFileSummary parsedFile typecheckedFile + let unusedBindingDiagnostics = fileSummary ^.. _Just . to termsBySymbol . folded . _3 . folding (UnusedBindings.analyseTerm fileUri) + Debug.debugM Debug.Temp "Unused binding diagnostics" unusedBindingDiagnostics let tokenMap = getTokenMap tokens conflictWarningDiagnostics <- fold <$> for fileSummary \fs -> lift $ computeConflictWarningDiagnostics fileUri fs let diagnosticRanges = - (errDiagnostics <> conflictWarningDiagnostics) + (errDiagnostics <> conflictWarningDiagnostics <> unusedBindingDiagnostics) & fmap (\d -> (d ^. range, d)) & toRangeMap let fileAnalysis = FileAnalysis {diagnostics = diagnosticRanges, codeActions = codeActionRanges, fileSummary, typeSignatureHints, ..} diff --git a/unison-cli/src/Unison/LSP/FileAnalysis/UnusedBindings.hs b/unison-cli/src/Unison/LSP/FileAnalysis/UnusedBindings.hs new file mode 100644 index 000000000..0a9e231c5 --- /dev/null +++ b/unison-cli/src/Unison/LSP/FileAnalysis/UnusedBindings.hs @@ -0,0 +1,32 @@ +module Unison.LSP.FileAnalysis.UnusedBindings (analyseTerm) where + +import Data.Foldable qualified as Foldable +import Data.Map qualified as Map +import Data.Set qualified as Set +import Language.LSP.Protocol.Types (Diagnostic) +import Language.LSP.Protocol.Types qualified as Lsp +import U.Core.ABT (ABT (..)) +import U.Core.ABT qualified as ABT +import Unison.LSP.Conversions qualified as Cv +import Unison.LSP.Diagnostics qualified as Diagnostic +import Unison.Parser.Ann (Ann) +import Unison.Prelude +import Unison.Symbol (Symbol) +import Unison.Term (Term) + +analyseTerm :: Lsp.Uri -> Term Symbol Ann -> [Diagnostic] +analyseTerm fileUri tm = + let (unusedVars, _) = ABT.cata alg tm + in Map.toList unusedVars & mapMaybe \(v, ann) -> do + range <- Cv.annToRange ann + pure $ Diagnostic.mkDiagnostic fileUri range Diagnostic.DiagnosticSeverity_Warning ("Unused binding " <> tShow v) [] + where + alg :: (Foldable f, Ord v) => Ann -> ABT f v (Map v Ann, Set v) -> (Map v Ann, Set v) + alg ann abt = case abt of + Var v -> (mempty, Set.singleton v) + Cycle x -> x + Abs v (unusedBindings, usedVars) -> + if v `Set.member` usedVars + then (unusedBindings, Set.delete v usedVars) + else (Map.insert v ann unusedBindings, usedVars) + Tm fx -> Foldable.fold fx diff --git a/unison-cli/unison-cli.cabal b/unison-cli/unison-cli.cabal index a8b820276..94a05b1ea 100644 --- a/unison-cli/unison-cli.cabal +++ b/unison-cli/unison-cli.cabal @@ -128,6 +128,7 @@ library Unison.LSP.Conversions Unison.LSP.Diagnostics Unison.LSP.FileAnalysis + Unison.LSP.FileAnalysis.UnusedBindings Unison.LSP.FoldingRange Unison.LSP.Formatting Unison.LSP.HandlerUtils From 6a6c4d600481e697607d058555a8e02a2941472a Mon Sep 17 00:00:00 2001 From: Chris Penner Date: Fri, 5 Jul 2024 12:47:43 -0700 Subject: [PATCH 2/5] Swap to using para instead of cata --- .../Unison/LSP/FileAnalysis/UnusedBindings.hs | 22 +++++++++++-------- 1 file changed, 13 insertions(+), 9 deletions(-) diff --git a/unison-cli/src/Unison/LSP/FileAnalysis/UnusedBindings.hs b/unison-cli/src/Unison/LSP/FileAnalysis/UnusedBindings.hs index 0a9e231c5..7aee4d37a 100644 --- a/unison-cli/src/Unison/LSP/FileAnalysis/UnusedBindings.hs +++ b/unison-cli/src/Unison/LSP/FileAnalysis/UnusedBindings.hs @@ -1,8 +1,8 @@ module Unison.LSP.FileAnalysis.UnusedBindings (analyseTerm) where -import Data.Foldable qualified as Foldable -import Data.Map qualified as Map +import Control.Lens import Data.Set qualified as Set + import Language.LSP.Protocol.Types (Diagnostic) import Language.LSP.Protocol.Types qualified as Lsp import U.Core.ABT (ABT (..)) @@ -13,20 +13,24 @@ import Unison.Parser.Ann (Ann) import Unison.Prelude import Unison.Symbol (Symbol) import Unison.Term (Term) +import Unison.Term qualified as Term analyseTerm :: Lsp.Uri -> Term Symbol Ann -> [Diagnostic] analyseTerm fileUri tm = - let (unusedVars, _) = ABT.cata alg tm - in Map.toList unusedVars & mapMaybe \(v, ann) -> do + let (unusedVars, _) = ABT.para alg tm + in unusedVars & mapMaybe \(v, ann) -> do range <- Cv.annToRange ann pure $ Diagnostic.mkDiagnostic fileUri range Diagnostic.DiagnosticSeverity_Warning ("Unused binding " <> tShow v) [] where - alg :: (Foldable f, Ord v) => Ann -> ABT f v (Map v Ann, Set v) -> (Map v Ann, Set v) + alg :: (Ord v) => Ann -> ABT (Term.F v a a) v (Term v Ann, ([(v, Ann)], Set v)) -> ([(v, Ann)], Set v) alg ann abt = case abt of Var v -> (mempty, Set.singleton v) - Cycle x -> x - Abs v (unusedBindings, usedVars) -> + Cycle (_t, x) -> x + Abs v (_, (unusedBindings, usedVars)) -> if v `Set.member` usedVars then (unusedBindings, Set.delete v usedVars) - else (Map.insert v ann unusedBindings, usedVars) - Tm fx -> Foldable.fold fx + else ((v, ann) : unusedBindings, usedVars) + Tm fx -> case fx of + Term.Let _isTop (lhsTerm, lx) (_rhsTerm, rx) -> + set (_1 . _head . _2) (ABT.annotation lhsTerm) lx <> rx + _ -> foldOf (folded . _2) fx From 25db6fbf05d0e4c68dc8a7b6a1cec624c10113e8 Mon Sep 17 00:00:00 2001 From: Chris Penner Date: Fri, 5 Jul 2024 13:18:44 -0700 Subject: [PATCH 3/5] Revert back to simple cata --- .../Unison/LSP/FileAnalysis/UnusedBindings.hs | 26 ++++++++----------- 1 file changed, 11 insertions(+), 15 deletions(-) diff --git a/unison-cli/src/Unison/LSP/FileAnalysis/UnusedBindings.hs b/unison-cli/src/Unison/LSP/FileAnalysis/UnusedBindings.hs index 7aee4d37a..114d4a369 100644 --- a/unison-cli/src/Unison/LSP/FileAnalysis/UnusedBindings.hs +++ b/unison-cli/src/Unison/LSP/FileAnalysis/UnusedBindings.hs @@ -1,8 +1,8 @@ -module Unison.LSP.FileAnalysis.UnusedBindings (analyseTerm) where +module Unison.LSP.FileAnalysis.UnusedBindings where -import Control.Lens +import Data.Foldable qualified as Foldable +import Data.Map qualified as Map import Data.Set qualified as Set - import Language.LSP.Protocol.Types (Diagnostic) import Language.LSP.Protocol.Types qualified as Lsp import U.Core.ABT (ABT (..)) @@ -13,24 +13,20 @@ import Unison.Parser.Ann (Ann) import Unison.Prelude import Unison.Symbol (Symbol) import Unison.Term (Term) -import Unison.Term qualified as Term analyseTerm :: Lsp.Uri -> Term Symbol Ann -> [Diagnostic] analyseTerm fileUri tm = - let (unusedVars, _) = ABT.para alg tm - in unusedVars & mapMaybe \(v, ann) -> do + let (unusedVars, _) = ABT.cata alg tm + in Map.toList unusedVars & mapMaybe \(v, ann) -> do range <- Cv.annToRange ann pure $ Diagnostic.mkDiagnostic fileUri range Diagnostic.DiagnosticSeverity_Warning ("Unused binding " <> tShow v) [] where - alg :: (Ord v) => Ann -> ABT (Term.F v a a) v (Term v Ann, ([(v, Ann)], Set v)) -> ([(v, Ann)], Set v) + alg :: (Foldable f, Ord v) => Ann -> ABT f v (Map v Ann, Set v) -> (Map v Ann, Set v) alg ann abt = case abt of Var v -> (mempty, Set.singleton v) - Cycle (_t, x) -> x - Abs v (_, (unusedBindings, usedVars)) -> + Cycle x -> x + Abs v (unusedBindings, usedVars) -> if v `Set.member` usedVars - then (unusedBindings, Set.delete v usedVars) - else ((v, ann) : unusedBindings, usedVars) - Tm fx -> case fx of - Term.Let _isTop (lhsTerm, lx) (_rhsTerm, rx) -> - set (_1 . _head . _2) (ABT.annotation lhsTerm) lx <> rx - _ -> foldOf (folded . _2) fx + then (mempty, Set.delete v usedVars) + else (Map.insert v ann unusedBindings, usedVars) + Tm fx -> Foldable.fold fx From 61287bdce5d106636e6d165fc4bdbf424d75ca4c Mon Sep 17 00:00:00 2001 From: Chris Penner Date: Fri, 5 Jul 2024 13:19:13 -0700 Subject: [PATCH 4/5] Move warnings to the top of the term --- unison-cli/src/Unison/LSP/Conversions.hs | 7 +++++ unison-cli/src/Unison/LSP/FileAnalysis.hs | 3 +- .../Unison/LSP/FileAnalysis/UnusedBindings.hs | 29 ++++++++++++++----- 3 files changed, 30 insertions(+), 9 deletions(-) diff --git a/unison-cli/src/Unison/LSP/Conversions.hs b/unison-cli/src/Unison/LSP/Conversions.hs index 307fd5c99..f6163485c 100644 --- a/unison-cli/src/Unison/LSP/Conversions.hs +++ b/unison-cli/src/Unison/LSP/Conversions.hs @@ -49,3 +49,10 @@ annToRange = \case Ann.External -> Nothing Ann.GeneratedFrom a -> annToRange a Ann.Ann start end -> Just $ Range (uToLspPos start) (uToLspPos end) + +annToURange :: Ann.Ann -> Maybe Range.Range +annToURange = \case + Ann.Intrinsic -> Nothing + Ann.External -> Nothing + Ann.GeneratedFrom a -> annToURange a + Ann.Ann start end -> Just $ Range.Range start end diff --git a/unison-cli/src/Unison/LSP/FileAnalysis.hs b/unison-cli/src/Unison/LSP/FileAnalysis.hs index 32a51af4a..ddd7fc477 100644 --- a/unison-cli/src/Unison/LSP/FileAnalysis.hs +++ b/unison-cli/src/Unison/LSP/FileAnalysis.hs @@ -112,8 +112,7 @@ checkFile doc = runMaybeT do & toRangeMap let typeSignatureHints = fromMaybe mempty (mkTypeSignatureHints <$> parsedFile <*> typecheckedFile) let fileSummary = FileSummary.mkFileSummary parsedFile typecheckedFile - let unusedBindingDiagnostics = fileSummary ^.. _Just . to termsBySymbol . folded . _3 . folding (UnusedBindings.analyseTerm fileUri) - Debug.debugM Debug.Temp "Unused binding diagnostics" unusedBindingDiagnostics + let unusedBindingDiagnostics = fileSummary ^.. _Just . to termsBySymbol . folded . folding (\(topLevelAnn, _refId, trm, _type) -> UnusedBindings.analyseTerm fileUri topLevelAnn trm) let tokenMap = getTokenMap tokens conflictWarningDiagnostics <- fold <$> for fileSummary \fs -> diff --git a/unison-cli/src/Unison/LSP/FileAnalysis/UnusedBindings.hs b/unison-cli/src/Unison/LSP/FileAnalysis/UnusedBindings.hs index 114d4a369..acb5d15c3 100644 --- a/unison-cli/src/Unison/LSP/FileAnalysis/UnusedBindings.hs +++ b/unison-cli/src/Unison/LSP/FileAnalysis/UnusedBindings.hs @@ -3,6 +3,7 @@ module Unison.LSP.FileAnalysis.UnusedBindings where import Data.Foldable qualified as Foldable import Data.Map qualified as Map import Data.Set qualified as Set +import Data.Text qualified as Text import Language.LSP.Protocol.Types (Diagnostic) import Language.LSP.Protocol.Types qualified as Lsp import U.Core.ABT (ABT (..)) @@ -11,22 +12,36 @@ import Unison.LSP.Conversions qualified as Cv import Unison.LSP.Diagnostics qualified as Diagnostic import Unison.Parser.Ann (Ann) import Unison.Prelude -import Unison.Symbol (Symbol) +import Unison.Symbol (Symbol (..)) import Unison.Term (Term) +import Unison.Util.Range qualified as Range +import Unison.Var qualified as Var +import Unison.Lexer.Pos qualified as Pos -analyseTerm :: Lsp.Uri -> Term Symbol Ann -> [Diagnostic] -analyseTerm fileUri tm = +analyseTerm :: Lsp.Uri -> Ann -> Term Symbol Ann -> [Diagnostic] +analyseTerm fileUri topLevelTermAnn tm = let (unusedVars, _) = ABT.cata alg tm - in Map.toList unusedVars & mapMaybe \(v, ann) -> do - range <- Cv.annToRange ann - pure $ Diagnostic.mkDiagnostic fileUri range Diagnostic.DiagnosticSeverity_Warning ("Unused binding " <> tShow v) [] + in Map.toList unusedVars & mapMaybe \(v, _ann) -> do + name <- getRelevantVarName v + -- Unfortunately we don't capture the annotation of the actual binding when parsing :'(, for now the least + -- annoying thing to do is just highlight the top of the binding. + urange <- Cv.annToURange topLevelTermAnn <&> (\(Range.Range start@(Pos.Pos line _col) _end) -> Range.Range start (Pos.Pos line 9999)) + let lspRange = Cv.uToLspRange urange + pure $ Diagnostic.mkDiagnostic fileUri lspRange Diagnostic.DiagnosticSeverity_Warning ("Unused binding " <> tShow name <> " inside this term.\nUse the binding, or prefix it with an _ to dismiss this warning.") [] where + getRelevantVarName :: Symbol -> Maybe Text + getRelevantVarName = \case + -- We only care about user bindings which don't start with an underscore + Symbol _ (Var.User n) -> do + guard (not (Text.isPrefixOf "_" n)) + pure n + _ -> Nothing alg :: (Foldable f, Ord v) => Ann -> ABT f v (Map v Ann, Set v) -> (Map v Ann, Set v) alg ann abt = case abt of Var v -> (mempty, Set.singleton v) Cycle x -> x Abs v (unusedBindings, usedVars) -> if v `Set.member` usedVars - then (mempty, Set.delete v usedVars) + then (unusedBindings, Set.delete v usedVars) else (Map.insert v ann unusedBindings, usedVars) Tm fx -> Foldable.fold fx From a6b8af1edecff9f8028a30464b9bf4c83f2d09cb Mon Sep 17 00:00:00 2001 From: ChrisPenner Date: Fri, 5 Jul 2024 20:51:54 +0000 Subject: [PATCH 5/5] automatically run ormolu --- unison-cli/src/Unison/LSP/FileAnalysis/UnusedBindings.hs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/unison-cli/src/Unison/LSP/FileAnalysis/UnusedBindings.hs b/unison-cli/src/Unison/LSP/FileAnalysis/UnusedBindings.hs index acb5d15c3..1f2372930 100644 --- a/unison-cli/src/Unison/LSP/FileAnalysis/UnusedBindings.hs +++ b/unison-cli/src/Unison/LSP/FileAnalysis/UnusedBindings.hs @@ -10,13 +10,13 @@ import U.Core.ABT (ABT (..)) import U.Core.ABT qualified as ABT import Unison.LSP.Conversions qualified as Cv import Unison.LSP.Diagnostics qualified as Diagnostic +import Unison.Lexer.Pos qualified as Pos import Unison.Parser.Ann (Ann) import Unison.Prelude import Unison.Symbol (Symbol (..)) import Unison.Term (Term) import Unison.Util.Range qualified as Range import Unison.Var qualified as Var -import Unison.Lexer.Pos qualified as Pos analyseTerm :: Lsp.Uri -> Ann -> Term Symbol Ann -> [Diagnostic] analyseTerm fileUri topLevelTermAnn tm =