Extension pragma inserted below ghc options pragma (or shebang) no matter where it is in file

[eddiemundo]

Steps to reproduce

1. Have a ghc options pragma (or shebang) in the middle of the file somewhere. Both cases seem to compile fine...
2. Disable an extension that the file needs.
3. Use a code action to insert the extension back in.

Expected behaviour

The extension should be put after the last pragma at the top of the file somewhere before the module declaration.

Actual behaviour

The extension is inserted after the ghc options pragma (or shebang) in the middle of the file.

Working on a fix for this so that my fix for something else can work.

[jneira]

Many thanks for the report and for working in fixing it.
Position of pragmas has been, somewhat surprisingly, a tricky thing, there were several prs correcting different bugs (sometimes introduced by previous prs).

So i think add a regression test will be a must: there are already lot of them 😄

haskell-language-server/plugins/hls-pragmas-plugin/test/Main.hs

Lines 29 to 55 in 07042d4

	testGroup "code actions"
	[ codeActionTest "adds LANGUAGE with no other pragmas at start ignoring later INLINE pragma" "AddPragmaIgnoreInline" [("Add \"TupleSections\"", "Contains TupleSections code action")]
	, codeActionTest "adds LANGUAGE after shebang preceded by other LANGUAGE and GHC_OPTIONS" "AddPragmaAfterShebangPrecededByLangAndOptsGhc" [("Add \"TupleSections\"", "Contains TupleSections code action")]
	, codeActionTest "adds LANGUAGE after shebang with other Language preceding shebang" "AddPragmaAfterShebangPrecededByLangAndOptsGhc" [("Add \"TupleSections\"", "Contains TupleSections code action")]
	, codeActionTest "adds LANGUAGE before Doc comments after interchanging pragmas" "BeforeDocInterchanging" [("Add \"NamedFieldPuns\"", "Contains NamedFieldPuns code action")]
	, codeActionTest "Add language after altering OPTIONS_GHC and Language" "AddLanguagePragmaAfterInterchaningOptsGhcAndLangs" [("Add \"TupleSections\"", "Contains TupleSections code action")]
	, codeActionTest "Add language after pragmas with non standard space between prefix and name" "AddPragmaWithNonStandardSpacingInPrecedingPragmas" [("Add \"TupleSections\"", "Contains TupleSections code action")]
	, codeActionTest "adds LANGUAGE after OptGHC at start ignoring later INLINE pragma" "AddPragmaAfterOptsGhcIgnoreInline" [("Add \"TupleSections\"", "Contains TupleSections code action")]
	, codeActionTest "adds LANGUAGE ignore later Ann pragma" "AddPragmaIgnoreLaterAnnPragma" [("Add \"BangPatterns\"", "Contains BangPatterns code action")]
	, codeActionTest "adds LANGUAGE after interchanging pragmas ignoring later Ann pragma" "AddLanguageAfterInterchaningIgnoringLaterAnn" [("Add \"BangPatterns\"", "Contains BangPatterns code action")]
	, codeActionTest "adds LANGUAGE after OptGHC preceded by another language pragma" "AddLanguageAfterLanguageThenOptsGhc" [("Add \"NamedFieldPuns\"", "Contains NamedFieldPuns code action")]
	, codeActionTest "adds LANGUAGE pragma after shebang and last language pragma" "AfterShebangAndPragma" [("Add \"NamedFieldPuns\"", "Contains NamedFieldPuns code action")]
	, codeActionTest "adds above module keyword on first line" "ModuleOnFirstLine" [("Add \"TupleSections\"", "Contains TupleSections code action")]
	, codeActionTest "adds LANGUAGE pragma after GHC_OPTIONS" "AfterGhcOptions" [("Add \"TupleSections\"", "Contains TupleSections code action")]
	, codeActionTest "adds LANGUAGE pragma after shebang and GHC_OPTIONS" "AfterShebangAndOpts" [("Add \"TupleSections\"", "Contains TupleSections code action")]
	, codeActionTest "adds LANGUAGE pragma after shebang, GHC_OPTIONS and language pragma" "AfterShebangAndOptionsAndPragma" [("Add \"TupleSections\"", "Contains TupleSections code action")]
	, codeActionTest "adds LANGUAGE pragma after all others ignoring later INLINE pragma" "AfterShebangAndOptionsAndPragmasIgnoreInline" [("Add \"TupleSections\"", "Contains TupleSections code action")]
	, codeActionTest "adds LANGUAGE pragma after all others ignoring multiple later INLINE pragma" "AfterAllWithMultipleInlines" [("Add \"TupleSections\"", "Contains TupleSections code action")]
	, codeActionTest "adds LANGUAGE pragma correctly ignoring later INLINE pragma" "AddLanguagePragma" [("Add \"TupleSections\"", "Contains TupleSections code action")]
	, codeActionTest "adds TypeApplications pragma" "TypeApplications" [("Add \"TypeApplications\"", "Contains TypeApplications code action")]
	, codeActionTest "after shebang" "AfterShebang" [("Add \"NamedFieldPuns\"", "Contains NamedFieldPuns code action")]
	, codeActionTest "append to existing pragmas" "AppendToExisting" [("Add \"NamedFieldPuns\"", "Contains NamedFieldPuns code action")]
	, codeActionTest "before doc comments" "BeforeDocComment" [("Add \"NamedFieldPuns\"", "Contains NamedFieldPuns code action")]
	, codeActionTest "before doc comments" "MissingSignatures" [("Disable \"missing-signatures\" warnings", "Contains missing-signatures code action")]
	, codeActionTest "before doc comments" "UnusedImports" [("Disable \"unused-imports\" warnings", "Contains unused-imports code action")]
	, codeActionTest "adds TypeSynonymInstances pragma" "NeedsPragmas" [("Add \"TypeSynonymInstances\"", "Contains TypeSynonymInstances code action"), ("Add \"FlexibleInstances\"", "Contains FlexibleInstances code action")]
	]

prs: #2043, #2078 amongs other

[eddiemundo]

Finished writing a fix for this but there is a test that fails that I'm not sure is correct.

Test output was different from 'test/testdata/AddPragmaAfterShebangPrecededByLangAndOptsGhc.expected.hs'. Output of ["git","-c","core.fileMode=false","diff","--no-index","--text","--exit-code","test/testdata/AddPragmaAfterShebangPrecededByLangAndOptsGhc.expected.hs","/tmp/AddPragmaAfterShebangPrecededByLangAndOptsGhc.expected932384-1.actual"]:
      diff --git a/test/testdata/AddPragmaAfterShebangPrecededByLangAndOptsGhc.expected.hs b/tmp/AddPragmaAfterShebangPrecededByLangAndOptsGhc.expected932384-1.actual
      index b29fc22f..ba4ecd18 100644
      --- a/test/testdata/AddPragmaAfterShebangPrecededByLangAndOptsGhc.expected.hs
      +++ b/tmp/AddPragmaAfterShebangPrecededByLangAndOptsGhc.expected932384-1.actual
      @@ -1,8 +1,8 @@
       {-# LANGUAGE OverloadedStrings #-}
       {-# OPTIONS_GHC -Wall #-}
      +{-# LANGUAGE TupleSections #-}
       #! /usr/bin/env nix-shell
       #! nix-shell --pure -i runghc -p "haskellPackages.ghcWithPackages (hp: with hp; [ turtle ])"
      -{-# LANGUAGE TupleSections #-}
       
       data Something = Something {
           foo :: !String,

In this one it expects the pragma after the shebang lines but the shebang lines are after existing pragmas which makes them screwy to begin with since shebangs should be at the top of the file. So is this behaviour necessary? It's easy to change my implementation to match, but if the behaviour is not necessary it's better to delete the test.

[nini-faroux]

hi @eddiemundo

Thanks for the fix, I didn't think of the case of pragmas in the middle of the file.

With the tests I was just trying different combinations to be sure it was added under the last file header pragma, but if your output makes sense and passes your new tests as well I'm sure it's ok to ignore this one.