ormolu icon indicating copy to clipboard operation
ormolu copied to clipboard

formatting does not preserve position of comments inside explicit imports

Open epsilonhalbe opened this issue 4 years ago • 5 comments

formatting

import MyModule ({- instance MyClass -})

will produce

import MyModule ()

{- instance MyClass -}

which is not quite what I would expect but in multiple imports this is still clear that MyClass belongs to MyModule unfortunately the following is not so clear anymore

import           MyReallyLongModuleNameModuleB ({- instance AnotherClassB -})
import           MyReallyLongModuleNameA       ({- instance MyReallyLongClassNameA -})

import qualified MyModuleC                     as C
import qualified MyModuleD                     as D

will be formatted as

{- instance AnotherClassB -}
{- instance MyReallyLongClassNameA -}

import qualified MyModuleC as C
import qualified MyModuleD as D
import MyReallyLongModuleNameA ()
import MyReallyLongModuleNameModuleB ()

my assumption is that this is due to the length of class+module name

epsilonhalbe avatar Nov 28 '19 17:11 epsilonhalbe

Ormolu doesn't take length of lines into account. This problem you reported happens because those per-import comments are not attached to their imports, unlike Haddocks. Yet, we sort the imports in the import section, so we would need to sort comments as well to preserve the correspondence, which is a bit tricky to do with our current approach, but doable.

mrkkrp avatar Nov 29 '19 10:11 mrkkrp

In this style of code the comments are also not preserved:

import           Protolude

-- base
import           Control.Monad (when)
import           Control.Exception (bracket)

-- GLFW-b, qualified for clarity
import qualified Graphics.UI.GLFW as GLFW

-- gl, all types and funcs here will already start with "gl"
import           Graphics.GL.Core33
import           Graphics.GL.Types

With Ormolu the result is:

-- base

import Control.Exception (bracket)
import Control.Monad (when)
-- GLFW-b, qualified for clarity

-- gl, all types and funcs here will already start with "gl"
import Graphics.GL.Core33
import Graphics.GL.Types
import qualified Graphics.UI.GLFW as GLFW
import Protolude

And I wanted to ask if is it possible to preserve the order. I would prefer Protolude to be the first.

rainbyte avatar Jul 18 '20 22:07 rainbyte

I strongly support the claim of @rainbyte that when a programmer lets blank lines or comments in the middle of their import list, they do it voluntarily and it carries some information that should not be erased by the formatter.

GuillaumeGen avatar Nov 26 '21 16:11 GuillaumeGen

I agree with @GuillaumeGen and @rainbyte and also strongly support respecting empty lines in import blocks. I'd also like to raise a particularly bad consequence of Ormolu's current behavior.

Say we're writing a library that extends QuickCheck, which means lots of our modules will share the Test.QuickCheck path. Say the programmer wrote:

module X where

-- Qualified due to complex reasons described in this comment
import qualified Test.QuickCheck.Exception as QCE
import Test.QuickCheck.Features
import Test.QuickCheck.Function

-- important comment about cyclic dependencies or something
import Test.QuickCheck.DecisionTreeTest.Types
import Test.QuickCheck.DecisionTreeTest.Impl
import DecisionTree.API

If we run this through Ormolu, we get:

module X where

-- Qualified due to complex reasons described in this comment

-- important comment about cyclic dependencies or something

import DecisionTree.API
import Test.QuickCheck.DecisionTreeTest.Impl
import Test.QuickCheck.DecisionTreeTest.Types
import qualified Test.QuickCheck.Exception as QCE
import Test.QuickCheck.Features
import Test.QuickCheck.Function

Not only I lost the separation of imports, my imports got mixed up with QuickCheck's because they share a qualified name prefix but I also completely lost the meaning of the two important comments. This example was crafted to showcase this happening in a minimal fashion, but we actually had this problem in our codebase and unfortunately lost important comments attached to imports.

It could be reasonable to define the rules as:

import X
import Y

import A

gets grouped and sorted, since space by itself is not enough indication the programmer had any special thoughts; But,

import X
import Y

-- Important comment! I thought about this
import A

Is left as is. The space AND comment clearly indicates a motive here. The tricky bit happens when we have to decide what to do with:

import X
import Y

-- Important comment! I thought about this
import A

import B

I'd actually be happy with grouping and sorting B in either import block; if I want to keep it separate, I'll place another comment. Conservatively, could make sense to group it to the A import block since its the closes to it.

Tagging @amesgen here as we spoke about this today.

VictorCMiraldo avatar Dec 09 '21 10:12 VictorCMiraldo

I would like to use Ormolu in a codebase, but this is a blocker for now. Agree with @VictorCMiraldo's proposed solution.

emhoracek avatar Feb 15 '22 23:02 emhoracek