Skip to content

Fail upon encountering unrecognized fields in the config #1272

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
merged 12 commits into from
Sep 12, 2024
Merged

Fail upon encountering unrecognized fields in the config #1272

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
864be87
Fail upon encountering unrecognized fields in the config
fsoikin Aug 23, 2024
0e6ccb2
Changelog
fsoikin Aug 23, 2024
440984e
Remove bogus field
fsoikin Aug 23, 2024
2126309
Resistance is futile
fsoikin Aug 23, 2024
f5665c0
Merge remote-tracking branch 'upstream/master' into fail-on-bogus-fields
fsoikin Sep 7, 2024
cbde7e0
Update upstream libraries
fsoikin Sep 9, 2024
631f31e
Oops, revert accidental spago.yaml change
fsoikin Sep 9, 2024
c092b40
Restore the holy JSON
fsoikin Sep 9, 2024
4ceab91
Merge remote-tracking branch 'upstream/master' into fail-on-bogus-fields
fsoikin Sep 9, 2024
d83d89d
Duplicate publish.location codec
fsoikin Sep 10, 2024
4036115
Remove duplicate import
fsoikin Sep 11, 2024
1194b32
Update registry-lib ref
fsoikin Sep 11, 2024
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
2 changes: 2 additions & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,8 @@ Other improvements:
current workspace.
- #1110: `spago publish` will now install packages returned by the registry solver
before trying to build with them.
- Spago no longer ignores config fields that it doesn't recognize. This should
help catch typos in field names.

## [0.21.0] - 2023-05-04

Expand Down
205 changes: 125 additions & 80 deletions core/src/Config.purs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,31 +43,32 @@ import Data.Array.NonEmpty as NonEmptyArray
import Data.Codec as Codec
import Data.Codec.JSON as CJ
import Data.Codec.JSON.Record as CJ.Record
import Data.Codec.JSON.Strict as CJS
import Data.Codec.JSON.Sum as CJ.Sum
import Data.Either as Either
import Data.List as List
import Data.Map as Map
import Data.Profunctor as Profunctor
import Partial.Unsafe (unsafeCrashWith)
import Registry.Internal.Codec as Internal.Codec
import Registry.Internal.Codec as Reg.Internal.Codec
import Registry.Internal.Parsing as Reg.Internal.Parsing
import Registry.License as License
import Registry.Location as Location
import Registry.PackageName as PackageName
import Registry.Range as Range
import Registry.Sha256 as Sha256
import Registry.Version as Version
import Type.Proxy (Proxy(..))

type Config =
{ package :: Maybe PackageConfig
, workspace :: Maybe WorkspaceConfig
}

configCodec :: CJ.Codec Config
configCodec = CJ.object
$ CJ.recordPropOptional (Proxy @"package") packageConfigCodec
$ CJ.recordPropOptional (Proxy @"workspace") workspaceConfigCodec
$ CJ.record
configCodec = CJS.objectStrict
$ CJS.recordPropOptional @"package" packageConfigCodec
$ CJS.recordPropOptional @"workspace" workspaceConfigCodec
$ CJS.record

type PackageConfig =
{ name :: PackageName
Expand All @@ -81,16 +82,16 @@ type PackageConfig =
}

packageConfigCodec :: CJ.Codec PackageConfig
packageConfigCodec = CJ.named "PackageConfig" $ CJ.object
$ CJ.recordProp (Proxy @"name") PackageName.codec
$ CJ.recordPropOptional (Proxy @"description") CJ.string
$ CJ.recordProp (Proxy @"dependencies") dependenciesCodec
$ CJ.recordPropOptional (Proxy @"build") packageBuildOptionsCodec
$ CJ.recordPropOptional (Proxy @"bundle") bundleConfigCodec
$ CJ.recordPropOptional (Proxy @"run") runConfigCodec
$ CJ.recordPropOptional (Proxy @"test") testConfigCodec
$ CJ.recordPropOptional (Proxy @"publish") publishConfigCodec
$ CJ.record
packageConfigCodec = CJ.named "PackageConfig" $ CJS.objectStrict
$ CJS.recordProp @"name" PackageName.codec
$ CJS.recordPropOptional @"description" CJ.string
$ CJS.recordProp @"dependencies" dependenciesCodec
$ CJS.recordPropOptional @"build" packageBuildOptionsCodec
$ CJS.recordPropOptional @"bundle" bundleConfigCodec
$ CJS.recordPropOptional @"run" runConfigCodec
$ CJS.recordPropOptional @"test" testConfigCodec
$ CJS.recordPropOptional @"publish" publishConfigCodec
$ CJS.record

type PublishConfig =
{ version :: Version
Expand All @@ -101,24 +102,62 @@ type PublishConfig =
}

publishConfigCodec :: CJ.Codec PublishConfig
publishConfigCodec = CJ.named "PublishConfig" $ CJ.object
$ CJ.recordProp (Proxy @"version") Version.codec
$ CJ.recordProp (Proxy @"license") License.codec
$ CJ.recordPropOptional (Proxy @"location") Location.codec
$ CJ.recordPropOptional (Proxy @"include") (CJ.array CJ.string)
$ CJ.recordPropOptional (Proxy @"exclude") (CJ.array CJ.string)
$ CJ.record
publishConfigCodec = CJ.named "PublishConfig" $ CJS.objectStrict
$ CJS.recordProp @"version" Version.codec
$ CJS.recordProp @"license" License.codec
$ CJS.recordPropOptional @"location" publishLocationCodec
$ CJS.recordPropOptional @"include" (CJ.array CJ.string)
$ CJS.recordPropOptional @"exclude" (CJ.array CJ.string)
$ CJS.record

-- This codec duplicates `Location.codec` from the Registry library, but with
-- strict parsing of fields, so that we would error out on unknown fields, thus
-- catching typos in field names. We do not want to modify the original codec in
-- the Registry library, because it's used for network communication, not for
-- reading user input, and therefore it's more important there to ignore unknown
-- fields for backwards compatibiility.
publishLocationCodec :: CJ.Codec Location
publishLocationCodec = CJ.named "Publish Location" $ Codec.codec' decode encode
where
decode json =
(Location.Git <$> Codec.decode gitCodec json)
<|> (Location.GitHub <$> Codec.decode githubCodec json)

encode = case _ of
Location.Git git -> CJ.encode gitCodec git
Location.GitHub github -> CJ.encode githubCodec github

githubCodec :: CJ.Codec Location.GitHubData
githubCodec = Profunctor.dimap toJsonRep fromJsonRep $ CJ.named "GitHub" $ CJ.Record.objectStrict
{ githubOwner: CJ.string
, githubRepo: CJ.string
, subdir: CJ.Record.optional CJ.string
}
where
toJsonRep { owner, repo, subdir } = { githubOwner: owner, githubRepo: repo, subdir }
fromJsonRep { githubOwner, githubRepo, subdir } = { owner: githubOwner, repo: githubRepo, subdir }

gitCodec :: CJ.Codec Location.GitData
gitCodec = Profunctor.dimap toJsonRep fromJsonRep $ CJ.named "Git" $ CJ.Record.objectStrict
{ gitUrl: Reg.Internal.Codec.parsedString Reg.Internal.Parsing.gitUrl
, subdir: CJ.Record.optional CJ.string
}
where
-- The JSON representation of the GitHub type uses 'gitUrl', but in PureScript
-- we use 'url' for convenience.
toJsonRep { url, subdir } = { gitUrl: url, subdir }
fromJsonRep { gitUrl, subdir } = { url: gitUrl, subdir }

type RunConfig =
{ main :: Maybe String
, execArgs :: Maybe (Array String)
}

runConfigCodec :: CJ.Codec RunConfig
runConfigCodec = CJ.named "RunConfig" $ CJ.object
$ CJ.recordPropOptional (Proxy @"main") CJ.string
$ CJ.recordPropOptional (Proxy @"execArgs") (CJ.array CJ.string)
$ CJ.record
runConfigCodec = CJ.named "RunConfig" $ CJS.objectStrict
$ CJS.recordPropOptional @"main" CJ.string
$ CJS.recordPropOptional @"execArgs" (CJ.array CJ.string)
$ CJS.record

type TestConfig =
{ main :: String
Expand All @@ -130,25 +169,25 @@ type TestConfig =
}

testConfigCodec :: CJ.Codec TestConfig
testConfigCodec = CJ.named "TestConfig" $ CJ.object
$ CJ.recordProp (Proxy @"main") CJ.string
$ CJ.recordPropOptional (Proxy @"execArgs") (CJ.array CJ.string)
$ CJ.recordPropOptional (Proxy @"censorTestWarnings") censorBuildWarningsCodec
$ CJ.recordPropOptional (Proxy @"strict") CJ.boolean
$ CJ.recordPropOptional (Proxy @"pedanticPackages") CJ.boolean
$ CJ.recordProp (Proxy @"dependencies") dependenciesCodec
$ CJ.record
testConfigCodec = CJ.named "TestConfig" $ CJS.objectStrict
$ CJS.recordProp @"main" CJ.string
$ CJS.recordPropOptional @"execArgs" (CJ.array CJ.string)
$ CJS.recordPropOptional @"censorTestWarnings" censorBuildWarningsCodec
$ CJS.recordPropOptional @"strict" CJ.boolean
$ CJS.recordPropOptional @"pedanticPackages" CJ.boolean
$ CJS.recordProp @"dependencies" dependenciesCodec
$ CJS.record

type BackendConfig =
{ cmd :: String
, args :: Maybe (Array String)
}

backendConfigCodec :: CJ.Codec BackendConfig
backendConfigCodec = CJ.named "BackendConfig" $ CJ.object
$ CJ.recordProp (Proxy @"cmd") CJ.string
$ CJ.recordPropOptional (Proxy @"args") (CJ.array CJ.string)
$ CJ.record
backendConfigCodec = CJ.named "BackendConfig" $ CJS.objectStrict
$ CJS.recordProp @"cmd" CJ.string
$ CJS.recordPropOptional @"args" (CJ.array CJ.string)
$ CJS.record

type PackageBuildOptionsInput =
{ censorProjectWarnings :: Maybe CensorBuildWarnings
Expand All @@ -157,11 +196,11 @@ type PackageBuildOptionsInput =
}

packageBuildOptionsCodec :: CJ.Codec PackageBuildOptionsInput
packageBuildOptionsCodec = CJ.named "PackageBuildOptionsInput" $ CJ.object
$ CJ.recordPropOptional (Proxy @"censorProjectWarnings") censorBuildWarningsCodec
$ CJ.recordPropOptional (Proxy @"strict") CJ.boolean
$ CJ.recordPropOptional (Proxy @"pedanticPackages") CJ.boolean
$ CJ.record
packageBuildOptionsCodec = CJ.named "PackageBuildOptionsInput" $ CJS.objectStrict
$ CJS.recordPropOptional @"censorProjectWarnings" censorBuildWarningsCodec
$ CJS.recordPropOptional @"strict" CJ.boolean
$ CJS.recordPropOptional @"pedanticPackages" CJ.boolean
$ CJS.record

type BundleConfig =
{ minify :: Maybe Boolean
Expand All @@ -173,17 +212,19 @@ type BundleConfig =
}

bundleConfigCodec :: CJ.Codec BundleConfig
bundleConfigCodec = CJ.named "BundleConfig" $ CJ.object
$ CJ.recordPropOptional (Proxy @"minify") CJ.boolean
$ CJ.recordPropOptional (Proxy @"module") CJ.string
$ CJ.recordPropOptional (Proxy @"outfile") CJ.string
$ CJ.recordPropOptional (Proxy @"platform") bundlePlatformCodec
$ CJ.recordPropOptional (Proxy @"type") bundleTypeCodec
$ CJ.recordPropOptional (Proxy @"extraArgs") (CJ.array CJ.string)
$ CJ.record
bundleConfigCodec = CJ.named "BundleConfig" $ CJS.objectStrict
$ CJS.recordPropOptional @"minify" CJ.boolean
$ CJS.recordPropOptional @"module" CJ.string
$ CJS.recordPropOptional @"outfile" CJ.string
$ CJS.recordPropOptional @"platform" bundlePlatformCodec
$ CJS.recordPropOptional @"type" bundleTypeCodec
$ CJS.recordPropOptional @"extraArgs" (CJ.array CJ.string)
$ CJS.record

data BundlePlatform = BundleNode | BundleBrowser

derive instance Eq BundlePlatform

instance Show BundlePlatform where
show = case _ of
BundleNode -> "node"
Expand All @@ -202,6 +243,8 @@ bundlePlatformCodec = CJ.Sum.enumSum show (parsePlatform)
-- App bundles with a main fn, while Module does not include a main.
data BundleType = BundleApp | BundleModule

derive instance Eq BundleType

instance Show BundleType where
show = case _ of
BundleApp -> "app"
Expand Down Expand Up @@ -238,7 +281,7 @@ instance Monoid Dependencies where
dependenciesCodec :: CJ.Codec Dependencies
dependenciesCodec = Profunctor.dimap to from $ CJ.array dependencyCodec
where
packageSingletonCodec = Internal.Codec.packageMap spagoRangeCodec
packageSingletonCodec = Reg.Internal.Codec.packageMap spagoRangeCodec

to :: Dependencies -> Array (Either PackageName (Map PackageName Range))
to (Dependencies deps) =
Expand Down Expand Up @@ -291,12 +334,12 @@ type WorkspaceConfig =
}

workspaceConfigCodec :: CJ.Codec WorkspaceConfig
workspaceConfigCodec = CJ.named "WorkspaceConfig" $ CJ.object
$ CJ.recordPropOptional (Proxy @"packageSet") setAddressCodec
$ CJ.recordPropOptional (Proxy @"backend") backendConfigCodec
$ CJ.recordPropOptional (Proxy @"buildOpts") buildOptionsCodec
$ CJ.recordPropOptional (Proxy @"extraPackages") (Internal.Codec.packageMap extraPackageCodec)
$ CJ.record
workspaceConfigCodec = CJ.named "WorkspaceConfig" $ CJS.objectStrict
$ CJS.recordPropOptional @"packageSet" setAddressCodec
$ CJS.recordPropOptional @"backend" backendConfigCodec
$ CJS.recordPropOptional @"buildOpts" buildOptionsCodec
$ CJS.recordPropOptional @"extraPackages" (Reg.Internal.Codec.packageMap extraPackageCodec)
$ CJS.record

type WorkspaceBuildOptionsInput =
{ output :: Maybe FilePath
Expand All @@ -305,11 +348,11 @@ type WorkspaceBuildOptionsInput =
}

buildOptionsCodec :: CJ.Codec WorkspaceBuildOptionsInput
buildOptionsCodec = CJ.named "WorkspaceBuildOptionsInput" $ CJ.object
$ CJ.recordPropOptional (Proxy @"output") CJ.string
$ CJ.recordPropOptional (Proxy @"censorLibraryWarnings") censorBuildWarningsCodec
$ CJ.recordPropOptional (Proxy @"statVerbosity") statVerbosityCodec
$ CJ.record
buildOptionsCodec = CJ.named "WorkspaceBuildOptionsInput" $ CJS.objectStrict
$ CJS.recordPropOptional @"output" CJ.string
$ CJS.recordPropOptional @"censorLibraryWarnings" censorBuildWarningsCodec
$ CJS.recordPropOptional @"statVerbosity" statVerbosityCodec
$ CJS.record

data CensorBuildWarnings
= CensorAllWarnings
Expand Down Expand Up @@ -361,13 +404,15 @@ warningCensorTestCodec = Codec.codec' decode encode
byCode = ByCode <$> Codec.decode CJ.string json
byPrefix = (ByMessagePrefix <<< _.byPrefix) <$> Codec.decode byMessagePrefixCodec json

byMessagePrefixCodec = CJ.named "ByMessagePrefix" $ CJ.Record.object { byPrefix: CJ.string }
byMessagePrefixCodec = CJ.named "ByMessagePrefix" $ CJ.Record.objectStrict { byPrefix: CJ.string }

data StatVerbosity
= NoStats
| CompactStats
| VerboseStats

derive instance Eq StatVerbosity

instance Show StatVerbosity where
show = case _ of
NoStats -> "NoStats"
Expand Down Expand Up @@ -397,9 +442,9 @@ derive instance Eq SetAddress
setAddressCodec :: CJ.Codec SetAddress
setAddressCodec = Codec.codec' decode encode
where
setFromRegistryCodec = CJ.named "SetFromRegistry" $ CJ.Record.object { registry: Version.codec }
setFromUrlCodec = CJ.named "SetFromUrl" $ CJ.Record.object { url: CJ.string, hash: CJ.Record.optional Sha256.codec }
setFromPathCodec = CJ.named "SetFromPath" $ CJ.Record.object { path: CJ.string }
setFromRegistryCodec = CJ.named "SetFromRegistry" $ CJ.Record.objectStrict { registry: Version.codec }
setFromUrlCodec = CJ.named "SetFromUrl" $ CJ.Record.objectStrict { url: CJ.string, hash: CJ.Record.optional Sha256.codec }
setFromPathCodec = CJ.named "SetFromPath" $ CJ.Record.objectStrict { path: CJ.string }

encode (SetFromRegistry r) = CJ.encode setFromRegistryCodec r
encode (SetFromUrl u) = CJ.encode setFromUrlCodec u
Expand Down Expand Up @@ -427,7 +472,7 @@ extraPackageCodec = Codec.codec' decode encode
type LocalPackage = { path :: FilePath }

localPackageCodec :: CJ.Codec LocalPackage
localPackageCodec = CJ.named "LocalPackage" $ CJ.Record.object { path: CJ.string }
localPackageCodec = CJ.named "LocalPackage" $ CJ.Record.objectStrict { path: CJ.string }

data RemotePackage
= RemoteGitPackage GitPackage
Expand Down Expand Up @@ -455,12 +500,12 @@ type GitPackage =
}

gitPackageCodec :: CJ.Codec GitPackage
gitPackageCodec = CJ.named "GitPackage" $ CJ.object
$ CJ.recordProp (Proxy @"git") CJ.string
$ CJ.recordProp (Proxy @"ref") CJ.string
$ CJ.recordPropOptional (Proxy @"subdir") CJ.string
$ CJ.recordPropOptional (Proxy @"dependencies") dependenciesCodec
$ CJ.record
gitPackageCodec = CJ.named "GitPackage" $ CJS.objectStrict
$ CJS.recordProp @"git" CJ.string
$ CJS.recordProp @"ref" CJ.string
$ CJS.recordPropOptional @"subdir" CJ.string
$ CJS.recordPropOptional @"dependencies" dependenciesCodec
$ CJS.record

-- | The format of a legacy packages.json package set entry for an individual
-- | package.
Expand All @@ -471,8 +516,8 @@ type LegacyPackageSetEntry =
}

legacyPackageSetEntryCodec :: CJ.Codec LegacyPackageSetEntry
legacyPackageSetEntryCodec = CJ.named "LegacyPackageSetEntry" $ CJ.object
$ CJ.recordProp (Proxy @"repo") CJ.string
$ CJ.recordProp (Proxy @"version") CJ.string
$ CJ.recordProp (Proxy @"dependencies") (CJ.array PackageName.codec)
$ CJ.record
legacyPackageSetEntryCodec = CJ.named "LegacyPackageSetEntry" $ CJS.objectStrict
$ CJS.recordProp @"repo" CJ.string
$ CJS.recordProp @"version" CJ.string
$ CJS.recordProp @"dependencies" (CJ.array PackageName.codec)
$ CJS.record
2 changes: 1 addition & 1 deletion docs-search/client-halogen/spago.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,6 @@ package:
bundle:
type: "app"
minify: true
sourceMaps: true
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And immediately we have a winner!

Turns out spago bundle has a command-line parameter for source maps, but not a config parameter. Another bug.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Or not a bug? Technically this can be specified in extra args, as I did here, but this is asymmetry. Not sure what the overarching philosophy is here.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great catch!

Overarching philosophy is that we try to keep flags and config params as close to each other as possible. There are some notable exceptions, but this one is a bug: we should have a config parameter as well.

module: Docs.Search.App
outfile: "../../bin/docs-search-app.js"
platform: browser
Expand All @@ -47,3 +46,4 @@ package:
# The node module is also considered deprecated and recommends using the upstream npm package punycode. So its an easy swap-in.
# The extra / at the end is how you tell node and esbuild to override a builtin node package with a user-space package.
- "--alias:punycode=punycode/"
- "--sourcemap"
Loading
Loading