Warning about unqualified implicit imports

changed weight to 5

added Tfeature request Trac import labels

I'm afraid I don't understand the specification. Could you write a specification of the feature you want?

Simon

My attempt for a specification:

Add the option -fwarn-unqualified-imports that make GHC do the following: Whenever GHC encounters import statements of one of the following types

module Main where

import Top.A
import Top.A hiding (x, y, z)
import Top.A as A
import Top.A as A hiding (x, y, z)

it shall emit a warning like:

Main.hs:3:0:
    Warning: Unqualified implicit import of module Top.A
             which risks name collisions if identifiers are added to Top.A in the future.
             Better write
                import qualified Top.A as A
             or
                import Top.A (x, y, z, ...)

Ideally the second suggested import statement (import Top.A (...)) contains all identifiers from Top.A that are actually used in Main. Then the suggested import statement can be copied into Main and Main can be compiled without further modifications.

The option -Wall may not include this warning, since some people prefer those kinds of imports.

mentioned in issue #4980

Hmm. So what triggers the warning? All your examples are for a module Top.A; would the warning happen if I say import M? It seems odd if the module name is influential... and these days almost all module names have dots in them.

Maybe you meant to warn of any import that brings into scope a bunch of unqualified names, unless they are explicitly enumerated. What about

import Top.A( T(..) )

Is that warned about too? What is the general rule?

Simon

Replying to [ticket:4977#comment:48913 simonpj]:

Hmm. So what triggers the warning? All your examples are for a module Top.A; would the warning happen if I say import M?

The module name Top.A is irrelevant. 'import M' shall also trigger the warning.

Maybe you meant to warn of any import that brings into scope a bunch of unqualified names, unless they are explicitly enumerated. What about
import Top.A( T(..) )
Is that warned about too?

Ah, I missed that. Yes, importing constructors and class methods unqualified and without explicit enumeration should also cause a warning.

What is the general rule?

My intention is to warn about all import statements, that can cause the module to break, if there is something added to the imported module. Removals and modifications of the API of imported modules are out of scope for any warning, since we cannot protect ourselves against such changes via the choice of import statements.

I brought the PVP into the discussion, since if you import module M with qualification or with explicit identifier list and M is part of the package p and p follows the Package Versioning Policy, then you can declare

Build-Depends: p == A.B.*

in your Cabal file. If you use imports that bring a bunch of identifiers into scope without qualification and explicit enumeration, then you have to restrict the version range to

Build-Depends: p == A.B.C.*

Thus this warning could promote the PVP and could encourage people to import in a way that allows larger import version ranges and thus reduce the number of updates to packages.

Doesn't -fwarn-missing-import-lists do what you want?

Replying to [ticket:4977#comment:49025 simonmar]:

Doesn't -fwarn-missing-import-lists do what you want?

I did not know about this option, since it is not listed in:

http://www.haskell.org/ghc/docs/7.0-latest/html/users_guide/options-sanity.html

Now that you mention it, I could find it in http://www.haskell.org/ghc/docs/7.0-latest/html/users_guide/flag-reference.html#id3106416 and its short description sounds, like it could be useful for me, but I am not entirely sure.

Thus at least I'd like to read a more detailed description of that flag.

I believe it does exactly what you want - that is, it warns about

import M
import M hiding (x,y)

but not

import M (x,y)

The documentation is missing though, so we should fix that before closing this ticket.

changed milestone to %7.2.1

changed weight to 10

Trac metadata

Trac field	Value
Priority	normal → highest

mentioned in commit 4b7d8ae0

assigned to @trac-igloo

In an odd moment I wrote the documentation. I've also changed the behaviour slightly to warn only for unqualified imports. I agree with Lemming that this is the case we really want to warn about.

I attach the patch because I can't make the documentation on my laptop and I don't want to break the built by a formatting error. Ian can you check and apply?

Simon

Attached file T4977.patch (download).

closed

Applied, thanks!

Trac metadata

Trac field	Value
Resolution	Unresolved → ResolvedFixed

added Phighest label

Trac field	Value
Version	7.0.1
Type	FeatureRequest
TypeOfFailure	OtherFailure
Priority	normal
Resolution	Unresolved
Component	Compiler
Test case
Differential revisions
BlockedBy
Related
Blocking
CC	ghc@henning-thielemann.de
Operating system
Architecture

Warning about unqualified implicit imports

Child items 0

Activity