Skip to content

Commit

Permalink
use weeder (#2044)
Browse files Browse the repository at this point in the history
Closes #2043

# Local usage notes

I suspect that HLS in VS Code spontaneously pollutes the `.hie` directory, because after a few successful invocations, `weeder` started complaining with:
```
incompatible hie file: /home/kostmo/github/swarm/.hie/Swarm/Web/Auth.hie
    this version of weeder was compiled with GHC version 9064
    the hie files in this project were generated with GHC version 9048
    weeder must be built with the same GHC version as the project it is used on
```

## Fixing false positives

Previously, for each `library` and `executable`, the HIE directory was always the same:
```
ghc-options:
    -hiedir=.hie
```
However, most of the executables have at least one module path that is the same relative to their `hs-source-dirs` base: namely, `Main.hs`.  This resulted in all but one of the `Main.hie` files being overwritten in the end.

To avoid this, I have specified a different `-hiedir` for each `library`/`executable`/`test` that parallels its `hs-source-dirs` base.  This way, so long as `hs-source-dirs` are unique across these targets, all of the `*.hs` files across the entire project will have unique `*.hie` paths.

## Whitelisting exceptions

There are some known limitations with `weeder` regarding support for Template Haskell and type families.  Exceptions are specified in the `roots` list in `weeder.toml`.

After removing a handful of dead functions myself, there are approx. 30 "true positive" weeds that I have listed explicitly in `weeder.toml`.  Maintenance of this list of exceptions should eventually be easier with ocharles/weeder#146.

# Integration with CI

I found a ready-made GitHub Action for weeder: https://github.com/freckle/weeder-action
I hacked support directly into the generated `.github/workflows/haskell-ci.yml` file.

Ideally, the generator would have an option for a `weeder` step.  Indeed, there is an open issue for supporting `weeder` in `haskell-ci`: haskell-CI/haskell-ci#132

A separate but related functionality that would be nice in `haskell-ci` is to specify **one** of the GHC versions in the matrix to do additional validations/builds that would be redundant if performed on the other matrix entries.  I suppose `weeder` is inexpensive enough to redo four times, but the `weeder` binary is not available for download for all GHC versions (e.g. ghc `9.8.2`).  Something like `haddock` we probably only need to build once.
I have hacked this in to the generated file for `weeder` with a simple [`if` condition](https://github.com/swarm-game/swarm/pull/2044/files#diff-73f8a010e9b95aa9fe944c316576b12c2ec961272d428e38721112ecb1c1a98bR227).
  • Loading branch information
kostmo authored Jul 17, 2024
1 parent ff8fdaa commit 35975bb
Show file tree
Hide file tree
Showing 10 changed files with 145 additions and 24 deletions.
10 changes: 9 additions & 1 deletion .github/workflows/haskell-ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,8 @@
## When changing versions, do not forget to update .mergify.yaml #
## #
## WARNING: #
## Hand edited to add filter in on.push and on.pull_request. #
## Hand-edited to add filter in on.push and on.pull_request. #
## Also hand-edited to add 'weeder' step to GHC 9.6.5. #
## If possible do not manually edit beside the above. #
#####################################################################
#
Expand Down Expand Up @@ -220,6 +221,13 @@ jobs:
- name: build
run: |
$CABAL v2-build $ARG_COMPILER $ARG_TESTS $ARG_BENCH all --write-ghc-environment-files=always
- name: weeder
if: matrix.compilerVersion == '9.6.5'
uses: freckle/weeder-action@v2
with:
ghc-version: ${{ matrix.compilerVersion }}
weeder-arguments: --config $GITHUB_WORKSPACE/source/weeder.toml
working-directory: ${{ env.PKGDIR_swarm }}
- name: tests
run: |
$CABAL v2-test $ARG_COMPILER $ARG_TESTS $ARG_BENCH all --test-show-details=direct
Expand Down
File renamed without changes.
File renamed without changes.
11 changes: 11 additions & 0 deletions scripts/validate/weeder.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
#!/bin/bash -ex

cd $(git rev-parse --show-toplevel)

# First, install Weeder:
# cabal install weeder

cabal clean
cabal build -O0 -j all

weeder -N
3 changes: 0 additions & 3 deletions src/swarm-engine/Swarm/Game/Scenario/Status.hs
Original file line number Diff line number Diff line change
Expand Up @@ -67,9 +67,6 @@ instance ToJSON ScenarioStatus where
toEncoding = genericToEncoding scenarioOptions
toJSON = genericToJSON scenarioOptions

seedLaunchParams :: Applicative f => Maybe Seed -> ParameterizableLaunchParams a f
seedLaunchParams s = LaunchParams (pure s) (pure Nothing)

emptyLaunchParams :: Applicative f => ParameterizableLaunchParams a f
emptyLaunchParams = LaunchParams (pure Nothing) (pure Nothing)

Expand Down
5 changes: 0 additions & 5 deletions src/swarm-scenario/Swarm/Game/Scenario/Objective/Graph.hs
Original file line number Diff line number Diff line change
Expand Up @@ -53,11 +53,6 @@ instance ToSample GraphInfo where
deriving instance Generic (BE.Signed ObjectiveLabel)
deriving instance ToJSON (BE.Signed ObjectiveLabel)

getConstFromSigned :: BE.Signed a -> a
getConstFromSigned = \case
BE.Positive x -> x
BE.Negative x -> x

getDistinctConstants :: (Ord a) => Prerequisite a -> Set (BE.Signed a)
getDistinctConstants = Set.fromList . BE.constants . toBoolExpr

Expand Down
9 changes: 0 additions & 9 deletions src/swarm-topography/Swarm/Game/Location.hs
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@ module Swarm.Game.Location (
toDirection,
toAbsDirection,
nearestDirection,
fromDirection,
isCardinal,
north,
south,
Expand Down Expand Up @@ -192,14 +191,6 @@ nearestDirection coord =
index = round $ fromIntegral (length orderedDirs) * angle
orderedDirs = Util.enumerateNonEmpty

-- | Convert a 'Direction' into a corresponding 'Heading'. Note that
-- this only does something reasonable for 'DNorth', 'DSouth', 'DEast',
-- and 'DWest'---other 'Direction's return the zero vector.
fromDirection :: Direction -> Heading
fromDirection = \case
DAbsolute x -> toHeading x
_ -> zero

-- | Manhattan distance between world locations.
manhattan :: Location -> Location -> Int32
manhattan (Location x1 y1) (Location x2 y2) = abs (x1 - x2) + abs (y1 - y2)
Expand Down
3 changes: 0 additions & 3 deletions src/swarm-tui/Swarm/TUI/Editor/Model.hs
Original file line number Diff line number Diff line change
Expand Up @@ -41,9 +41,6 @@ toFacade = \case
Facade f -> f
Ref e -> mkFacade e

getEntityName :: EntityFacade -> E.EntityName
getEntityName (EntityFacade name _) = name

data MapEditingBounds = MapEditingBounds
{ _boundsRect :: Maybe (Cosmic BoundsRectangle)
-- ^ Upper-left and lower-right coordinates
Expand Down
59 changes: 56 additions & 3 deletions swarm.cabal
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,6 @@ common common
common stan-config
ghc-options:
-fwrite-ide-info
-hiedir=.hie

-- Harmless extensions from GHC2021
common ghc2021-extensions
Expand Down Expand Up @@ -199,6 +198,9 @@ library swarm-lang

build-depends: swarm:swarm-util
hs-source-dirs: src/swarm-lang
ghc-options:
-hiedir=.hie/src/swarm-lang

default-language: Haskell2010
default-extensions:
-- Avoid unexpected unevaluated thunk buildup
Expand Down Expand Up @@ -253,6 +255,9 @@ library swarm-topography
swarm:swarm-util

hs-source-dirs: src/swarm-topography
ghc-options:
-hiedir=.hie/src/swarm-topography

default-language: Haskell2010
default-extensions:
-- Avoid unexpected unevaluated thunk buildup
Expand Down Expand Up @@ -351,6 +356,9 @@ library swarm-scenario
swarm:swarm-util,

hs-source-dirs: src/swarm-scenario
ghc-options:
-hiedir=.hie/src/swarm-scenario

default-language: Haskell2010
default-extensions:
-- Avoid unexpected unevaluated thunk buildup
Expand Down Expand Up @@ -451,6 +459,9 @@ library swarm-engine
swarm:swarm-util,

hs-source-dirs: src/swarm-engine
ghc-options:
-hiedir=.hie/src/swarm-engine

default-language: Haskell2010
default-extensions:
-- Avoid unexpected unevaluated thunk buildup
Expand Down Expand Up @@ -497,6 +508,9 @@ library swarm-web
swarm:swarm-util,

hs-source-dirs: src/swarm-web
ghc-options:
-hiedir=.hie/src/swarm-web

default-language: Haskell2010
default-extensions:
-- Avoid unexpected unevaluated thunk buildup
Expand Down Expand Up @@ -555,6 +569,9 @@ library swarm-tournament
swarm:swarm-util,

hs-source-dirs: src/swarm-tournament
ghc-options:
-hiedir=.hie/src/swarm-tournament

default-language: Haskell2010

library swarm-util
Expand Down Expand Up @@ -604,6 +621,9 @@ library swarm-util
yaml >=0.11 && <0.11.12.0,

hs-source-dirs: src/swarm-util
ghc-options:
-hiedir=.hie/src/swarm-util

default-language: Haskell2010
default-extensions:
-- Avoid unexpected unevaluated thunk buildup
Expand Down Expand Up @@ -655,6 +675,9 @@ library swarm-doc
swarm:swarm-util,

hs-source-dirs: src/swarm-doc
ghc-options:
-hiedir=.hie/src/swarm-doc

default-language: Haskell2010
default-extensions:
-- Avoid unexpected unevaluated thunk buildup
Expand Down Expand Up @@ -761,14 +784,17 @@ library swarm-tui
swarm:swarm-util,

hs-source-dirs: src/swarm-tui
ghc-options:
-hiedir=.hie/src/swarm-tui

default-language: Haskell2010
default-extensions:
-- Avoid unexpected unevaluated thunk buildup
-- See discussion in #415
StrictData

executable swarm
import: stan-config, common
import: stan-config, common, ghc2021-extensions
main-is: Main.hs
other-modules: Swarm.App
build-depends:
Expand All @@ -789,7 +815,10 @@ executable swarm
vty,
vty-crossplatform >=0.4 && <0.5,

hs-source-dirs: app
hs-source-dirs: app/game
ghc-options:
-hiedir=.hie/app/game

default-language: Haskell2010
ghc-options: -threaded
default-extensions: ImportQualifiedPost
Expand All @@ -806,6 +835,9 @@ executable swarm-scene
swarm:swarm-topography,

hs-source-dirs: app/scene
ghc-options:
-hiedir=.hie/app/scene

default-language: Haskell2010
ghc-options: -threaded
default-extensions: ImportQualifiedPost
Expand All @@ -822,6 +854,9 @@ executable swarm-docs
text,

hs-source-dirs: app/doc
ghc-options:
-hiedir=.hie/app/doc

default-language: Haskell2010
ghc-options: -threaded
default-extensions: ImportQualifiedPost
Expand All @@ -842,6 +877,9 @@ executable swarm-host-tournament
swarm:swarm-tournament,

hs-source-dirs: app/tournament
ghc-options:
-hiedir=.hie/app/tournament

default-language: Haskell2010
ghc-options: -threaded
default-extensions: ImportQualifiedPost
Expand Down Expand Up @@ -899,6 +937,9 @@ test-suite swarm-unit
swarm:swarm-util,

hs-source-dirs: test/unit
ghc-options:
-hiedir=.hie/test/unit

default-language: Haskell2010
ghc-options: -threaded

Expand Down Expand Up @@ -931,6 +972,9 @@ test-suite swarm-integration
swarm:swarm-util,

hs-source-dirs: test/integration
ghc-options:
-hiedir=.hie/test/integration

default-language: Haskell2010
ghc-options: -threaded

Expand All @@ -954,6 +998,9 @@ test-suite tournament-host
swarm:swarm-tournament,

hs-source-dirs: test/tournament-host
ghc-options:
-hiedir=.hie/test/tournament-host

default-language: Haskell2010
ghc-options: -threaded

Expand Down Expand Up @@ -982,13 +1029,19 @@ test-suite standalone-topography
swarm:swarm-util,

hs-source-dirs: test/standalone-topography/src
ghc-options:
-hiedir=.hie/test/standalone-topography/src

default-language: Haskell2010
ghc-options: -threaded

benchmark benchmark
import: stan-config, common, ghc2021-extensions
main-is: Benchmark.hs
hs-source-dirs: test/bench
ghc-options:
-hiedir=.hie/test/bench

type: exitcode-stdio-1.0
build-depends:
base,
Expand Down
69 changes: 69 additions & 0 deletions weeder.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,69 @@
roots = [
"^Main.main$",
"^Paths_.*",

# Workarounds for TH parsing:
# -------------------------------
"^Swarm.Language.Pipeline.QQ.tmQ$",
"^Swarm.Language.Parser.QQ.tyQ$",
"^Swarm.Util.Lens.makeLensesNoSigs$",
"^Swarm.Util.Lens.makeLensesExcluding$",

# Workarounds for type families:
# -------------------------------
"^Swarm.Game.World.Compile.NoFunParams$",

# True positives:
# =====================================
"^Main.prop_hittingSetMinimal$",
"^Main.printAllLogs$",
"^Control.Carrier.Accum.FixedStrict.execAccum$",
"^Swarm.App.demoWeb$",
"^Swarm.Effect.Unify.Common.dom$",
"^Swarm.Effect.Unify.Fast.@@$",
"^Swarm.Effect.Unify.Naive.runUnification$",
"^Swarm.Game.Entity.entityNameFor$",
"^Swarm.Game.Entity.singleton$",
"^Swarm.Game.Entity.Cosmetic.getBackground$",
"^Swarm.Game.Step.traceLogShow$",
"^Swarm.Game.World.Compile.compile$",
"^Swarm.Game.World.Compile.runCTerm$",
"^Swarm.Game.World.lookupTerrainM$",
"^Swarm.Language.Context.withBindings$",
"^Swarm.Language.Context.singleton$",
"^Swarm.Language.Parser.Util.showShortError$",
"^Swarm.Language.Pipeline.extractTCtx$",
"^Swarm.Language.Pretty.Prec$",
"^Swarm.Language.Pretty.appliedTermPrec$",
"^Swarm.Language.Requirements.Type.insert$",
"^Swarm.Language.Syntax.Pattern.UTerm$",
"^Swarm.Language.Syntax.Util.asTree$",
"^Swarm.Language.Syntax.Util.mapFreeS$",
"^Swarm.Util.replaceLast$",
"^Swarm.Util.reflow$",
"^Swarm.Util.isSuccessOr$",
"^Swarm.Util._NonEmpty$",

# True positives (unused lenses):
# -------------------------------
"^Swarm.Language.Typed.polytype$",
"^Swarm.Language.Typed.requires$",
"^Swarm.Language.Typed.value$",
"^Swarm.Language.Value.emptyEnv$",
"^Swarm.Game.Scenario.staticPlacements$",
"^Swarm.Game.Scenario.structureDefs$",
"^Swarm.Game.Scenario.Scoring.Best.scenarioBestByAstSize$",
"^Swarm.Game.Scenario.Scoring.Best.scenarioBestByCharCount$",
"^Swarm.Game.Scenario.Scoring.Best.scenarioBestByTicks$",
"^Swarm.Game.Scenario.Scoring.Best.scenarioBestByTime$",
"^Swarm.Game.ScenarioInfo._NotStarted$",
"^Swarm.Game.ScenarioInfo._Played$",
"^Swarm.Game.State.Robot._VCLocation$",
"^Swarm.Game.State.Robot._VCRobot$",
"^Swarm.Game.State.Substate._NoWinCondition$",
"^Swarm.Game.State.Substate._WinConditions$",
]

type-class-roots = true
unused-types = true

0 comments on commit 35975bb

Please sign in to comment.