refactor: eliminate Prelude.head/tail partial functions + redundant foldl' imports#55
Merged
Merged
Conversation
HLS flagged Prelude.head and Prelude.tail in five modules. All call sites had a non-empty-list invariant from the surrounding code, so the fixes preserve behaviour exactly: - Parser1.hs / Parser2.hs: psPath pops on XML close tags replace 'tail (psPath state)' with 'drop 1 (psPath state)'. Total on empty, identical on non-empty, matches the defensive 'if null then [] else tail' the file already used in its otherwise branch. - UnitConversion.hs: mergeUnitConfigs' non-empty equation binds the head via pattern match instead of head. - Upload.hs: pickByFileCount / findMethodDirectory pattern-match on the sorted result; the impossible empty case returns the parent dir. - CrossLinking.hs: fold the outer 'if null allCandidates' into a case so the no-name-match branch binds the supplier directly.
foldl' is re-exported from Prelude since base 4.20 (GHC 9.10). The explicit imports in ChemSynonyms, Types, and BM25 were flagged redundant by HLS once the project moved to GHC 9.12.4 (PR #52).
All suggestions HLS surfaced on the files modified earlier in this PR: - Drop unused LANGUAGE pragmas: LambdaCase (Parser1, Parser2), BangPatterns / RecordWildCards (UnitConversion). - Eta-reduce: 'cdata = text' (Parser1 x2, Parser2), 'removeZeroAmountCoproducts = filter keepExchange' (Parser1, Parser2), 'modifyAt' (UnitConversion), 'findAllDataDirectories' / 'findAllMethodDirectories' (Upload), 'findSupplierAcrossDatabases' (CrossLinking). - Replace 'case Maybe of Just x -> x; Nothing -> def' with 'fromMaybe def m' across both parsers (activityLoc, psActivityName, psLocation, psRefUnit) and 'convertExchangeAmount' (UnitConversion). - '[x | Just x <- xs]' -> 'catMaybes xs' (Parser2 x2). - '[e | Left e <- rs]' / '[r | Right r <- rs]' -> 'lefts' / 'rights' (Parser1). - 'if cond then [msg] else []' -> '[msg | cond]' list comprehension (Parser2 x2). - Rewrite nested-if 'isInput' with guards (Parser2). - '\\k _ -> k' -> 'const' (UnitConversion). - 'any (== ext)' -> '`elem`' (Upload x4). No behaviour change: every rewrite is an equivalence.
…d imports)
Drives lib:volca to a warning-free build under GHC 9.12.4's -Wall:
- Partial fn -> total
- Database/Loader.hs:280: 'map head . group . sort' -> 'map NE.head .
NE.group . sort'.
- Method/FlowResolver.hs:308: head removed by pattern-matching the
non-empty case 'cats@(firstCat : _)'.
- Database/Manager.hs:2901-2903: 'head/tail/init/last parts' in
parseGeographiesCSV replaced by 'unsnoc parts' + pattern match on
'parts@(codeRaw : _)'.
- Database/Manager.hs:2363: 'head errs' eliminated by folding the
'null allMethods && not (null errs)' guard into a 'case (null
allMethods, errs)' match.
- Service.hs supply-chain path: replace 'head pids / tail pids' with
a 'Just pids@(firstPid : restPids)' match; add the impossible
'Just [] -> Left (ActivityNotFound ...)' branch.
- Dead code: drop unused 'resolveRegionalCF' and its sole caller
'firstJust' from Method/Mapping.hs.
- Duplicate / unused imports
- Method/Mapping.hs: drop duplicate 'qualified Data.Set as S' and
unused 'qualified Matrix' (the unqualified 'Vector' from
'Matrix (Inventory, Vector)' suffices).
- Method/ParserSimaPro.hs, SimaPro/Parser.hs: drop redundant Data.List
'foldl'' import (re-exported from Prelude since base 4.20 / GHC 9.10).
- Style touch-ups that fell out of the same hot patches
- Mapping.hs sumRegionalizedLCIAScoreCrossDB: use 'rights' / 'lefts'.
- Database/Manager.hs: 'any (== ext)' -> "ext" `elem`,
'[r | Just r <- ...]' -> 'catMaybes', list-comprehension
'Left'/'Right' filters -> 'lefts'/'rights', 'when (not ...)' ->
'unless'.
No behaviour change; every rewrite is an equivalence on non-empty inputs
and adds a safe total branch where the previous partial function would
have crashed.
Drives 'cabal build test:lca-tests' to a warning-free build:
volca.cabal
- Suppress four idiomatic-in-test warning categories on the test stanza
only ('-Wno-x-partial', '-Wno-incomplete-uni-patterns',
'-Wno-incomplete-record-updates', '-Wno-name-shadowing'). Test code
destructures well-formed fixtures with partial patterns ('let Just x
= parse fixture'); a fixture mismatch should crash the test with the
relevant source location rather than be silenced or hand-rewrapped
into 'expectationFailure' boilerplate in every spec.
- Drop the deprecated '-dev' version tag (cabal warns on tagged
versions); bump to 0.6.1.0 (4-segment PVP).
Test sources: remove dead cruft
- Unused imports: Data.UUID/UUID, Data.Map, Data.Text, GoldenData,
Method.Types (Compartment), System.IO, System.IO.Temp,
qualified UnitConversion, Database.Upload, computeScalingVector,
Data.Set, and a stale qualified Data.Text in SimaProParserSpec.
- Unused top-level bindings: 'f2'/'f3' (BM25Spec), 'fid3' (EcoSpold1Spec),
'isRight' helpers (ILCDParserSpec, UnitConversionSpec), 'actUUID2'
(LoaderSpec), 'findInput' / 'approxEqAlloc' (SimaProParserSpec). The
matching type signatures are trimmed in the same step.
- Unused local binding 'offDiagonalEntries' (MatrixExportSpec): drop
the dead 'let' along with the comment that referred to it.
- Unused matches: prefix '_' on 'rootSolver' setup binders in
CrossDBRegionalLCIASensitivitySpec, on 'ph' in ServerSpec, on
'label' in TestHelpers.assertVectorNear.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
HLS was flagging eleven partial-function uses (`Prelude.head` / `Prelude.tail`) and three redundant `foldl'` imports across the library. This PR clears all of them with behaviour-preserving edits — no API change, no new types, no semantic shift.
Commit 1 — partial functions
All eleven sites had a non-empty invariant from the surrounding code; the rewrites preserve behaviour exactly and remove the partiality from the types/match shapes.
Commit 2 — redundant imports
`foldl'` is re-exported from `Prelude` since base 4.20 (GHC 9.10). PR #52 moved the project to GHC 9.12.4, after which HLS flagged the explicit imports in `Method/ChemSynonyms`, `Types`, and `Search/BM25` as redundant. Dropped.
Verification
Test plan