Skip to content

Make explicit import suggestions more consistent #3142

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
kritzcreek merged 2 commits into from
Jan 20, 2018
Merged

Make explicit import suggestions more consistent #3142

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
41bc81a
Make explicit import suggestions more consistent
nwolverson Oct 30, 2017
abde990
Make ImplicitQualifiedImport suggestions consistent
nwolverson Dec 17, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
36 changes: 23 additions & 13 deletions src/Language/PureScript/Linter/Imports.hs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ lintImports (Module _ _ mn mdecls (Just mexports)) env usedImps = do
let names = ordNub $ M.findWithDefault [] mni usedImps'
usedRefs = findUsedRefs ss env mni (Just mnq) names
unless (null usedRefs) .
tell . errorMessage' ss $ ImplicitQualifiedImport mni mnq usedRefs
tell . errorMessage' ss $ ImplicitQualifiedImport mni mnq $ map (simplifyTypeRef $ const True) usedRefs

for_ imports $ \(mnq, imps) -> do

Expand Down Expand Up @@ -183,6 +183,17 @@ lintImports (Module _ _ mn mdecls (Just mexports)) env usedImps = do
_ -> internalError "unqualified name in extractByQual"
go _ = Nothing


-- Replace explicit type refs with data constructor lists from listing the
-- used constructors explicity `T(X, Y, [...])` to `T(..)` for suggestion
-- message.
-- Done everywhere when suggesting a completely new explicit imports list, otherwise
-- maintain the existing form.
simplifyTypeRef :: (ProperName 'TypeName -> Bool) -> DeclarationRef -> DeclarationRef
simplifyTypeRef shouldOpen (TypeRef ss name (Just dctors))
| not (null dctors) && shouldOpen name = TypeRef ss name Nothing
simplifyTypeRef _ other = other

lintImportDecl
:: forall m
. MonadWriter MultipleErrors m
Expand Down Expand Up @@ -214,15 +225,7 @@ lintImportDecl env mni qualifierName names ss declType allowImplicit =
checkImplicit warning =
if null allRefs
then unused
else warn (warning mni (map simplifyTypeRef allRefs))
where
-- Replace explicit type refs with data constructor lists from listing the
-- used constructors explicity `T(X, Y, [...])` to `T(..)` for suggestion
-- message.
simplifyTypeRef :: DeclarationRef -> DeclarationRef
simplifyTypeRef (TypeRef ss' name (Just dctors))
| not (null dctors) = TypeRef ss' name Nothing
simplifyTypeRef other = other
else warn (warning mni (map (simplifyTypeRef $ const True) allRefs))

checkExplicit
:: [DeclarationRef]
Expand All @@ -236,21 +239,28 @@ lintImportDecl env mni qualifierName names ss declType allowImplicit =
didWarn <- case (length diff, length idents) of
(0, _) -> return False
(n, m) | n == m -> unused
_ -> warn (UnusedExplicitImport mni diff qualifierName allRefs)
_ -> warn (UnusedExplicitImport mni diff qualifierName $ map simplifyTypeRef' allRefs)

didWarn' <- forM (mapMaybe getTypeRef declrefs) $ \(tn, c) -> do
let allCtors = dctorsForType mni tn
-- If we've not already warned a type is unused, check its data constructors
unless' (TyName tn `notElem` usedNames) $
case (c, dctors `intersect` allCtors) of
(_, []) | c /= Just [] -> warn (UnusedDctorImport mni tn qualifierName allRefs)
(_, []) | c /= Just [] -> warn (UnusedDctorImport mni tn qualifierName $ map simplifyTypeRef' allRefs)
(Just ctors, dctors') ->
let ddiff = ctors \\ dctors'
in unless' (null ddiff) . warn $ UnusedDctorExplicitImport mni tn ddiff qualifierName allRefs
in unless' (null ddiff) . warn $ UnusedDctorExplicitImport mni tn ddiff qualifierName $ map simplifyTypeRef' allRefs
_ -> return False

return (didWarn || or didWarn')

where
simplifyTypeRef' :: DeclarationRef -> DeclarationRef
simplifyTypeRef' = simplifyTypeRef (\name -> any (isMatch name) declrefs)
where
isMatch name (TypeRef _ name' Nothing) = name == name'
isMatch _ _ = False

unused :: m Bool
unused = warn (UnusedImport mni)

Expand Down