hls-notes-plugin: Initial implementation (#4126)

* hls-notes-plugin: Initial implementation

* hls-notes-plugin: add to feature list and plugin table

* hls-notes-plugin: Add more documentation comments

* hls-notes-plugin: Fix tests after #3846, add CI job

* hls-notes-plugin: Address review comments

* hls-notes-plugin: Allow Note definition within single line comments

* hls-notes-plugin: Improve "Note not found" error message

* hls-notes-plugin: Allow single line notes to be indented

* treewide: Add missing underscores to note definitions

* hls-notes-plugin: Wait until HLS is done in tests

* hls-notes-plugin: Fix tests on windows

The regex did not allow windows line endings in note definitions

---------

Co-authored-by: Jan Hrcek <[email protected]>
Co-authored-by: fendor <[email protected]>

Author: 3 people

.github/workflows/test.yml

@@ -74,7 +74,7 @@ jobs:
           - ubuntu-latest
           - macOS-latest
           - windows-latest
-        test: 
+        test:
           - true
           - false
         exclude:
@@ -112,140 +112,143 @@ jobs:
 
       - if: matrix.test
         name: Test hls-graph
-        run: cabal test hls-graph 
+        run: cabal test hls-graph
 
       - if: needs.pre_job.outputs.should_skip_ghcide != 'true' && matrix.test
         name: Test ghcide
         # run the tests without parallelism to avoid running out of memory
-        run: cabal test ghcide || cabal test ghcide 
+        run: cabal test ghcide || cabal test ghcide
 
       - if: matrix.test
         name: Test hls-plugin-api
-        run: cabal test hls-plugin-api || cabal test hls-plugin-api 
+        run: cabal test hls-plugin-api || cabal test hls-plugin-api
 
       - if: matrix.test
         name: Test func-test suite
         env:
           HLS_TEST_EXE: hls
           HLS_WRAPPER_TEST_EXE: hls-wrapper
-        run: cabal test func-test || cabal test func-test 
+        run: cabal test func-test || cabal test func-test
 
       - if: matrix.test
         name: Test wrapper-test suite
         env:
           HLS_TEST_EXE: hls
           HLS_WRAPPER_TEST_EXE: hls-wrapper
-        run: cabal test wrapper-test 
+        run: cabal test wrapper-test
 
       - if: matrix.test
         name: Test hls-refactor-plugin
-        run: cabal test hls-refactor-plugin-tests || cabal test hls-refactor-plugin-tests 
+        run: cabal test hls-refactor-plugin-tests || cabal test hls-refactor-plugin-tests
 
-      - if: matrix.test 
+      - if: matrix.test
         name: Test hls-floskell-plugin
-        run: cabal test hls-floskell-plugin-tests || cabal test hls-floskell-plugin-tests 
+        run: cabal test hls-floskell-plugin-tests || cabal test hls-floskell-plugin-tests
 
       - if: matrix.test
         name: Test hls-class-plugin
-        run: cabal test hls-class-plugin-tests || cabal test hls-class-plugin-tests 
+        run: cabal test hls-class-plugin-tests || cabal test hls-class-plugin-tests
 
       - if: matrix.test
         name: Test hls-pragmas-plugin
-        run: cabal test hls-pragmas-plugin-tests || cabal test hls-pragmas-plugin-tests 
+        run: cabal test hls-pragmas-plugin-tests || cabal test hls-pragmas-plugin-tests
 
       - if: matrix.test
         name: Test hls-eval-plugin
-        run: cabal test hls-eval-plugin-tests || cabal test hls-eval-plugin-tests 
+        run: cabal test hls-eval-plugin-tests || cabal test hls-eval-plugin-tests
 
       - if: matrix.test
         name: Test hls-splice-plugin
-        run: cabal test hls-splice-plugin-tests || cabal test hls-splice-plugin-tests 
+        run: cabal test hls-splice-plugin-tests || cabal test hls-splice-plugin-tests
 
       - if: matrix.test && matrix.ghc != '9.2'
         name: Test hls-stan-plugin
-        run: cabal test hls-stan-plugin-tests || cabal test hls-stan-plugin-tests 
+        run: cabal test hls-stan-plugin-tests || cabal test hls-stan-plugin-tests
 
       - if: matrix.test
         name: Test hls-stylish-haskell-plugin
-        run: cabal test hls-stylish-haskell-plugin-tests || cabal test hls-stylish-haskell-plugin-tests 
+        run: cabal test hls-stylish-haskell-plugin-tests || cabal test hls-stylish-haskell-plugin-tests
 
-      - if: matrix.test 
+      - if: matrix.test
         name: Test hls-ormolu-plugin
-        run: cabal test hls-ormolu-plugin-tests || cabal test hls-ormolu-plugin-tests 
+        run: cabal test hls-ormolu-plugin-tests || cabal test hls-ormolu-plugin-tests
 
-      - if: matrix.test 
+      - if: matrix.test
         name: Test hls-fourmolu-plugin
-        run: cabal test hls-fourmolu-plugin-tests || cabal test hls-fourmolu-plugin-tests 
+        run: cabal test hls-fourmolu-plugin-tests || cabal test hls-fourmolu-plugin-tests
 
       - if: matrix.test
         name: Test hls-explicit-imports-plugin test suite
-        run: cabal test hls-explicit-imports-plugin-tests || cabal test hls-explicit-imports-plugin-tests 
+        run: cabal test hls-explicit-imports-plugin-tests || cabal test hls-explicit-imports-plugin-tests
 
       - if: matrix.test
         name: Test hls-call-hierarchy-plugin test suite
-        run: cabal test hls-call-hierarchy-plugin-tests || cabal test hls-call-hierarchy-plugin-tests 
+        run: cabal test hls-call-hierarchy-plugin-tests || cabal test hls-call-hierarchy-plugin-tests
 
       - if: matrix.test && matrix.os != 'windows-latest'
         name: Test hls-rename-plugin test suite
-        run: cabal test hls-rename-plugin-tests || cabal test hls-rename-plugin-tests 
+        run: cabal test hls-rename-plugin-tests || cabal test hls-rename-plugin-tests
 
-      - if: matrix.test 
+      - if: matrix.test
         name: Test hls-hlint-plugin test suite
-        run: cabal test hls-hlint-plugin-tests || cabal test hls-hlint-plugin-tests 
+        run: cabal test hls-hlint-plugin-tests || cabal test hls-hlint-plugin-tests
 
       - if: matrix.test
         name: Test hls-module-name-plugin test suite
-        run: cabal test hls-module-name-plugin-tests || cabal test hls-module-name-plugin-tests 
+        run: cabal test hls-module-name-plugin-tests || cabal test hls-module-name-plugin-tests
 
       - if: matrix.test
         name: Test hls-alternate-number-format-plugin test suite
-        run: cabal test hls-alternate-number-format-plugin-tests || cabal test hls-alternate-number-format-plugin-tests 
+        run: cabal test hls-alternate-number-format-plugin-tests || cabal test hls-alternate-number-format-plugin-tests
 
       - if: matrix.test
         name: Test hls-qualify-imported-names-plugin test suite
-        run: cabal test hls-qualify-imported-names-plugin-tests || cabal test hls-qualify-imported-names-plugin-tests 
+        run: cabal test hls-qualify-imported-names-plugin-tests || cabal test hls-qualify-imported-names-plugin-tests
 
       - if: matrix.test
         name: Test hls-code-range-plugin test suite
-        run: cabal test hls-code-range-plugin-tests || cabal test hls-code-range-plugin-tests 
+        run: cabal test hls-code-range-plugin-tests || cabal test hls-code-range-plugin-tests
 
       - if: matrix.test
         name: Test hls-change-type-signature test suite
-        run: cabal test hls-change-type-signature-plugin-tests || cabal test hls-change-type-signature-plugin-tests 
+        run: cabal test hls-change-type-signature-plugin-tests || cabal test hls-change-type-signature-plugin-tests
 
       - if: matrix.test
         name: Test hls-gadt-plugin test suit
-        run: cabal test hls-gadt-plugin-tests || cabal test hls-gadt-plugin-tests 
+        run: cabal test hls-gadt-plugin-tests || cabal test hls-gadt-plugin-tests
 
       - if: matrix.test
         name: Test hls-explicit-fixity-plugin test suite
-        run: cabal test hls-explicit-fixity-plugin-tests || cabal test hls-explicit-fixity-plugin-tests 
+        run: cabal test hls-explicit-fixity-plugin-tests || cabal test hls-explicit-fixity-plugin-tests
 
       - if: matrix.test
         name: Test hls-explicit-record-fields-plugin test suite
-        run: cabal test hls-explicit-record-fields-plugin-tests || cabal test hls-explicit-record-fields-plugin-tests 
+        run: cabal test hls-explicit-record-fields-plugin-tests || cabal test hls-explicit-record-fields-plugin-tests
 
       ## version needs to be limited since the tests depend on cabal-fmt which only builds using specific ghc versions
       - if: matrix.test && matrix.ghc == '9.2'
         name: Test hls-cabal-fmt-plugin test suite
-        run: cabal test hls-cabal-fmt-plugin-tests --flag=isolateCabalfmtTests || cabal test hls-cabal-fmt-plugin-tests --flag=isolateCabalfmtTests 
+        run: cabal test hls-cabal-fmt-plugin-tests --flag=isolateCabalfmtTests || cabal test hls-cabal-fmt-plugin-tests --flag=isolateCabalfmtTests
 
       - if: matrix.test
         name: Test hls-cabal-plugin test suite
-        run: cabal test hls-cabal-plugin-tests || cabal test hls-cabal-plugin-tests 
+        run: cabal test hls-cabal-plugin-tests || cabal test hls-cabal-plugin-tests
 
       - if: matrix.test
         name: Test hls-retrie-plugin test suite
-        run: cabal test hls-retrie-plugin-tests || cabal test hls-retrie-plugin-tests 
+        run: cabal test hls-retrie-plugin-tests || cabal test hls-retrie-plugin-tests
 
       - if: matrix.test
         name: Test hls-overloaded-record-dot-plugin test suite
-        run: cabal test hls-overloaded-record-dot-plugin-tests || cabal test hls-overloaded-record-dot-plugin-tests 
+        run: cabal test hls-overloaded-record-dot-plugin-tests || cabal test hls-overloaded-record-dot-plugin-tests
 
       - if: matrix.test
         name: Test hls-semantic-tokens-plugin test suite
-        run: cabal test hls-semantic-tokens-plugin-tests || cabal test hls-semantic-tokens-plugin-tests 
+        run: cabal test hls-semantic-tokens-plugin-tests || cabal test hls-semantic-tokens-plugin-tests
 
+      - if: matrix.test
+        name: Test hls-notes-plugin test suite
+        run: cabal test hls-notes-plugin-tests || cabal test hls-notes-plugin-tests
 
   test_post_job:
     if: always()

CODEOWNERS

@@ -28,6 +28,7 @@
 /plugins/hls-gadt-plugin @July541
 /plugins/hls-hlint-plugin @eddiemundo
 /plugins/hls-module-name-plugin
+/plugins/hls-notes-plugin @jvanbruegge
 /plugins/hls-ormolu-plugin @georgefst
 /plugins/hls-overloaded-record-dot-plugin @joyfulmantis
 /plugins/hls-pragmas-plugin @eddiemundo

docs/features.md

@@ -81,6 +81,12 @@ Known limitations:
 
 - Only works for [local definitions](https://github.com/haskell/haskell-language-server/issues/708).
 
+## Jump to note definition
+
+Provided by: `hls-notes-plugin`
+
+Jump to the definition of a [GHC-style note](https://gitlab.haskell.org/ghc/ghc/-/wikis/commentary/coding-style#2-using-notes).
+
 ## Find references
 
 Provided by: `ghcide`

docs/support/plugin-support.md

@@ -56,6 +56,7 @@ For example, a plugin to provide a formatter which has itself been abandoned has
 | `hls-gadt-plugin`                   | 2    |                          |
 | `hls-hlint-plugin`                  | 2    |                          |
 | `hls-module-name-plugin`            | 2    |                          |
+| `hls-notes-plugin`                  | 2    |                          |
 | `hls-qualify-imported-names-plugin` | 2    |                          |
 | `hls-ormolu-plugin`                 | 2    |                          |
 | `hls-rename-plugin`                 | 2    |                          |

flake.nix

@@ -69,7 +69,7 @@
             (pkgs.haskell.lib.justStaticExecutables (pkgs.haskell.lib.dontCheck pkgs.haskellPackages.opentelemetry-extra))
             capstone
             # ormolu
-            # stylish-haskell
+            stylish-haskell
             pre-commit
             ] ++ lib.optionals (!stdenv.isDarwin)
                    [ # tracy has a build problem on macos.

ghcide/src/Development/IDE/Core/Compile.hs

@@ -436,6 +436,7 @@ tcRnModule hsc_env tc_helpers pmod = do
 
 
 -- Note [Clearing mi_globals after generating an iface]
+-- ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 -- GHC populates the mi_global field in interfaces for GHCi if we are using the bytecode
 -- interpreter.
 -- However, this field is expensive in terms of heap usage, and we don't use it in HLS
@@ -1366,7 +1367,7 @@ loadHieFile ncu f = do
 
 
 {- Note [Recompilation avoidance in the presence of TH]
-
+   ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 Most versions of GHC we currently support don't have a working implementation of
 code unloading for object code, and no version of GHC supports this on certain
 platforms like Windows. This makes it completely infeasible for interactive use,
@@ -1736,6 +1737,7 @@ pathToModuleName = mkModuleName . map rep
       rep c = c
 
 {- Note [Guidelines For Using CPP In GHCIDE Import Statements]
+   ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   GHCIDE's interface with GHC is extensive, and unfortunately, because we have
   to work with multiple versions of GHC, we have several files that need to use
   a lot of CPP. In order to simplify the CPP in the import section of every file

ghcide/src/Development/IDE/Core/FileExists.hs

@@ -40,6 +40,7 @@ import qualified System.Directory                      as Dir
 import qualified System.FilePath.Glob                  as Glob
 
 {- Note [File existence cache and LSP file watchers]
+   ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 Some LSP servers provide the ability to register file watches with the client, which will then notify
 us of file changes. Some clients can do this more efficiently than us, or generally it's a tricky
 problem
@@ -135,6 +136,7 @@ getFileExists :: NormalizedFilePath -> Action Bool
 getFileExists fp = use_ GetFileExists fp
 
 {- Note [Which files should we watch?]
+   ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 The watcher system gives us a lot of flexibility: we can set multiple watchers, and they can all watch on glob
 patterns.
 
@@ -201,6 +203,7 @@ fileExistsRulesFast recorder isWatched =
             else fileExistsSlow file
 
 {- Note [Invalidating file existence results]
+   ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 We have two mechanisms for getting file existence information:
 - The file existence cache
 - The VFS lookup

ghcide/src/Development/IDE/Core/RuleTypes.hs

@@ -512,6 +512,7 @@ makeLensesWith
     ''Splices
 
 {- Note [Client configuration in Rules]
+   ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 The LSP client configuration is stored by `lsp` for us, and is accesible in
 handlers through the LspT monad.
 

ghcide/src/Development/IDE/Core/Rules.hs

@@ -749,6 +749,7 @@ instance Default GhcSessionDepsConfig where
     }
 
 -- | Note [GhcSessionDeps]
+--   ~~~~~~~~~~~~~~~~~~~~~
 -- For a file 'Foo', GhcSessionDeps "Foo.hs" results in an HscEnv which includes
 -- 1. HomeModInfo's (in the HUG/HPT) for all modules in the transitive closure of "Foo", **NOT** including "Foo" itself.
 -- 2. ModSummary's (in the ModuleGraph) for all modules in the transitive closure of "Foo", including "Foo" itself.

ghcide/src/Development/IDE/GHC/Error.hs

@@ -91,6 +91,7 @@ realSrcLocToPosition real =
   Position (fromIntegral $ srcLocLine real - 1) (fromIntegral $ srcLocCol real - 1)
 
 -- Note [Unicode support]
+-- ~~~~~~~~~~~~~~~~~~~~~~
 -- the current situation is:
 -- LSP Positions use UTF-16 code units(Unicode may count as variable columns);
 -- GHC use Unicode code points(Unicode count as one column).

ghcide/src/Development/IDE/Plugin/HLS.hs

@@ -359,6 +359,7 @@ instance Monoid IdeNotificationHandlers where
   mempty = IdeNotificationHandlers mempty
 
 {- Note [Exception handling in plugins]
+   ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 Plugins run in LspM, and so have access to IO. This means they are likely to
 throw exceptions, even if only by accident or through calling libraries that
 throw exceptions. Ultimately, we're running a bunch of less-trusted IO code,

haskell-language-server.cabal

@@ -64,7 +64,9 @@ common pedantic
   if flag(pedantic)
     ghc-options:
       -Werror
-      -- Note [unused-packages] Some packages need CPP conditioned on MIN_VERSION_ghc(x,y,z).
+      -- Note [unused-packages]
+      -- ~~~~~~~~~~~~~~~~~~~~~~
+      -- Some packages need CPP conditioned on MIN_VERSION_ghc(x,y,z).
       -- MIN_VERSION_<pkg> is CPP macro that cabal defines only when <pkg> is declared as a dependency.
       -- But -Wunused-packages still reports it as unused dependency if it's not imported.
       -- For packages with such "unused" dependencies we demote -Wunused-packages error
@@ -1628,6 +1630,61 @@ test-suite hls-semantic-tokens-plugin-tests
     , data-default
     , row-types
 
+-----------------------------
+-- notes plugin
+-----------------------------
+
+flag notes
+  description: Enable notes plugin
+  default:     True
+  manual:      True
+
+common notes
+  if flag(notes)
+    build-depends: haskell-language-server:hls-notes-plugin
+    cpp-options: -Dhls_notes
+
+library hls-notes-plugin
+  import: defaults, pedantic, warnings
+  buildable: True
+  exposed-modules:
+    Ide.Plugin.Notes
+  hs-source-dirs:     plugins/hls-notes-plugin/src
+  build-depends:
+    , base >=4.12 && <5
+    , array
+    , ghcide == 2.7.0.0
+    , hls-graph == 2.7.0.0
+    , hls-plugin-api == 2.7.0.0
+    , lens
+    , lsp >=2.4
+    , mtl >= 2.2
+    , regex-tdfa >= 1.3.1
+    , text
+    , text-rope
+    , unordered-containers
+  default-extensions:
+      DataKinds
+    , DeriveAnyClass
+    , DerivingStrategies
+    , OverloadedStrings
+    , LambdaCase
+    , TypeFamilies
+
+test-suite hls-notes-plugin-tests
+  import:           defaults, pedantic, test-defaults, warnings
+  type:             exitcode-stdio-1.0
+  hs-source-dirs:   plugins/hls-notes-plugin/test
+  main-is:          NotesTest.hs
+  build-depends:
+    , base
+    , directory
+    , filepath
+    , ghcide:ghcide-test-utils
+    , haskell-language-server:hls-notes-plugin
+    , hls-test-utils == 2.7.0.0
+  default-extensions: OverloadedStrings
+
 ----------------------------
 ----------------------------
 -- HLS
@@ -1666,6 +1723,7 @@ library
                   , refactor
                   , overloadedRecordDot
                   , semanticTokens
+                  , notes
 
   exposed-modules:
     Ide.Arguments