Skip to content

Remove unlawful Ord instance and replace it by a compare function #3279

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
michaelpj merged 1 commit into from
Oct 11, 2022
Merged

Remove unlawful Ord instance and replace it by a compare function #3279

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
20 changes: 11 additions & 9 deletions plugins/hls-refactor-plugin/src/Development/IDE/Plugin/CodeAction.hs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1520,7 +1520,7 @@ suggestNewImport packageExportsMap ps fileContents Diagnostic{_message}
, Just (range, indent) <- newImportInsertRange ps fileContents
, extendImportSuggestions <- matchRegexUnifySpaces msg
"Perhaps you want to add ‘[^’]*’ to the import list in the import of ‘([^’]*)’"
= let suggestions = nubSort
= let suggestions = nubSortBy simpleCompareImportSuggestion
(constructNewImportSuggestions packageExportsMap (qual <|> qual', thingMissing) extendImportSuggestions) in
map (\(ImportSuggestion _ kind (unNewImport -> imp)) -> (imp, kind, TextEdit range (imp <> "\n" <> T.replicate indent " "))) suggestions
where
Expand All @@ -1529,7 +1529,7 @@ suggestNewImport _ _ _ _ = []

constructNewImportSuggestions
:: ExportsMap -> (Maybe T.Text, NotInScope) -> Maybe [T.Text] -> [ImportSuggestion]
constructNewImportSuggestions exportsMap (qual, thingMissing) notTheseModules = nubOrd
constructNewImportSuggestions exportsMap (qual, thingMissing) notTheseModules = nubOrdBy simpleCompareImportSuggestion
[ suggestion
| Just name <- [T.stripPrefix (maybe "" (<> ".") qual) $ notInScope thingMissing] -- strip away qualified module names from the unknown name
, identInfo <- maybe [] Set.toList $ Map.lookup name (getExportsMap exportsMap) -- look up the modified unknown name in the export map
Expand Down Expand Up @@ -1568,16 +1568,18 @@ constructNewImportSuggestions exportsMap (qual, thingMissing) notTheseModules =
| otherwise = 0
m = moduleNameText identInfo

-- | Implements a lexicographic order for import suggestions.
-- First compares the importance score in DESCENDING order.
-- If the scores are equal it compares the import names alphabetical order.
data ImportSuggestion = ImportSuggestion !Int !CodeActionKind !NewImport
deriving ( Eq )

instance Ord ImportSuggestion where
compare (ImportSuggestion s1 _ i1) (ImportSuggestion s2 _ i2)
| s1 == s2 = compare i1 i2
| otherwise = flip compare s1 s2
-- | Implements a lexicographic order for import suggestions that ignores the code action.
-- First it compares the importance score in DESCENDING order.
-- If the scores are equal it compares the import names alphabetical order.
--
-- TODO: this should be a correct Ord instance but CodeActionKind does not implement a Ord
-- which would lead to an unlawful Ord instance.
simpleCompareImportSuggestion :: ImportSuggestion -> ImportSuggestion -> Ordering
simpleCompareImportSuggestion (ImportSuggestion s1 _ i1) (ImportSuggestion s2 _ i2)
= flip compare s1 s2 <> compare i1 i2

newtype NewImport = NewImport {unNewImport :: T.Text}
deriving (Show, Eq, Ord)
Expand Down