Proper boolean options; enable-matcher; imply enable-stack

andreasabel · andreasabel · commit 76c6a05434b4 · 2023-05-03T20:10:29.000+02:00
The options `enable-stack`, `stack-no-global`, `stack-setup-ghc` and `disable-matcher` are now proper booleans only accepting `true` or `false`. Previously, they were true when set to some non-empty string, even when set to "false" or "off" etc. `disable-matcher` is deprecated in favour of a new positive form `enable-matcher`. `enable-stack` is now implied by setting another stack-related option, i.e., one of `stack-version`, `stack-no-global` and `stack-setup-ghc`. Previously, it was a prerequisite to these options. Contradictory options now give an error, such as `stack-no-global` with `ghc-version` or `enable-stack: false` with `stack-version`. Fixes: haskell/actions#142
diff --git a/README.md b/README.md
@@ -187,23 +187,18 @@ jobs:
 
 ## Inputs
 
-| Name                    | Description                                                                                                                                 | Type      | Default     |
-| ----------------------- | ------------------------------------------------------------------------------------------------------------------------------------------- | --------- | ----------- |
-| `ghc-version`           | GHC version to use, e.g. `9.2` or `9.2.5`.                                                                                                  | `string`  | `latest`    |
-| `cabal-version`         | Cabal version to use, e.g. `3.6`.                                                                                                           | `string`  | `latest`    |
-| `stack-version`         | Stack version to use, e.g. `latest`. Stack will only be installed if `enable-stack` is set.                                                 | `string`  | `latest`    |
-| `enable-stack`          | If set, will setup Stack.                                                                                                                   | "boolean" | false/unset |
-| `stack-no-global`       | If set, `enable-stack` must be set. Prevents installing GHC and Cabal globally.                                                             | "boolean" | false/unset |
-| `stack-setup-ghc`       | If set, `enable-stack` must be set. Runs stack setup to install the specified GHC. (Note: setting this does _not_ imply `stack-no-global`.) | "boolean" | false/unset |
-| `disable-matcher`       | If set, disables match messages from GHC as GitHub CI annotations.                                                                          | "boolean" | false/unset |
-| `cabal-update`          | If set to `false`, skip `cabal update` step.                                                                                                | `boolean` | `true`      |
-| `ghcup-release-channel` | If set, add a [release channel](https://www.haskell.org/ghcup/guide/#pre-release-channels) to ghcup.                                        | `URL`     | none        |
-
-Note: "boolean" types are set/unset, not true/false.
-That is, setting any "boolean" to a value other than the empty string (`""`) will be considered true/set.
-However, to avoid confusion and for forward compatibility, it is still recommended to **only use value `true` to set a "boolean" flag.**
-
-In contrast, a proper `boolean` input like `cabal-update` only accepts values `true` and `false`.
+| Name                    | Description                                                                                                                     | Type      | Default  |
+| ----------------------- | ------------------------------------------------------------------------------------------------------------------------------- | --------- | -------- |
+| `ghc-version`           | GHC version to use, e.g. `9.2` or `9.2.5`.                                                                                      | `string`  | `latest` |
+| `cabal-version`         | Cabal version to use, e.g. `3.6`.                                                                                               | `string`  | `latest` |
+| `stack-version`         | Stack version to use, e.g. `latest`. Implies `enable-stack`.                                                                    | `string`  | `latest` |
+| `enable-stack`          | Setup Stack. Implied by `stack-version`, `stack-no-global`, `stack-setup-ghc`.                                                  | `boolean` | `false`  |
+| `stack-no-global`       | Implies `enable-stack`. Prevents installing GHC and Cabal globally.                                                             | `boolean` | `false`  |
+| `stack-setup-ghc`       | Implies `enable-stack`. Runs stack setup to install the specified GHC. (Note: setting this does _not_ imply `stack-no-global`.) | `boolean` | `false`  |
+| `enable-matcher`        | Enable match messages from GHC as GitHub CI annotations.                                                                        | `boolean` | `true`   |
+| `disable-matcher`       | Disable match messages from GHC as GitHub CI annotations. (Legacy option, deprecated in favour of `enable-matcher`.)            | `boolean` | `false`  |
+| `cabal-update`          | Perform `cabal update` step. (Default if Cabal is enabled.)                                                                     | `boolean` | `true`   |
+| `ghcup-release-channel` | If set, add a [release channel](https://www.haskell.org/ghcup/guide/#pre-release-channels) to ghcup.                            | `URL`     | none     |
 
 ## Outputs
 
diff --git a/__tests__/find-haskell.test.ts b/__tests__/find-haskell.test.ts
@@ -43,6 +43,14 @@ describe('haskell/actions/setup', () => {
       forAllTools(t => expect(def(os)[t].supported).toBe(supported_versions[t]))
     ));
 
+  it('Setting enable-matcher to false disables matcher', () => {
+    forAllOS(os => {
+      const options = getOpts(def(os), os, {
+        'enable-matcher': 'false'
+      });
+      expect(options.general.matcher.enable).toBe(false);
+    });
+  });
   it('Setting disable-matcher to true disables matcher', () => {
     forAllOS(os => {
       const options = getOpts(def(os), os, {
@@ -51,6 +59,25 @@ describe('haskell/actions/setup', () => {
       expect(options.general.matcher.enable).toBe(false);
     });
   });
+  it('Setting both enable-matcher to false and disable-matcher to true disables matcher', () => {
+    forAllOS(os => {
+      const options = getOpts(def(os), os, {
+        'enable-matcher': 'false',
+        'disable-matcher': 'true'
+      });
+      expect(options.general.matcher.enable).toBe(false);
+    });
+  });
+  it('Setting both enable-matcher and disable-matcher to true errors', () => {
+    forAllOS(os =>
+      expect(() =>
+        getOpts(def(os), os, {
+          'enable-matcher': 'true',
+          'disable-matcher': 'true'
+        })
+      ).toThrow()
+    );
+  });
 
   it('getOpts grabs default general settings correctly from environment', () => {
     forAllOS(os => {
@@ -135,15 +162,35 @@ describe('haskell/actions/setup', () => {
     });
   });
 
-  it('Enabling stack-no-global without setting enable-stack errors', () => {
+  it('Enabling stack-no-global but disabling enable-stack errors', () => {
     forAllOS(os =>
-      expect(() => getOpts(def(os), os, {'stack-no-global': 'true'})).toThrow()
+      expect(() =>
+        getOpts(def(os), os, {
+          'stack-no-global': 'true',
+          'enable-stack': 'false'
+        })
+      ).toThrow()
     );
   });
 
-  it('Enabling stack-setup-ghc without setting enable-stack errors', () => {
+  it('Enabling stack-no-global but setting ghc-version errors', () => {
+    forAllOS(os =>
+      expect(() =>
+        getOpts(def(os), os, {
+          'stack-no-global': 'true',
+          'ghc-version': 'latest'
+        })
+      ).toThrow()
+    );
+  });
+  it('Enabling stack-no-global but setting cabal-version errors', () => {
     forAllOS(os =>
-      expect(() => getOpts(def(os), os, {'stack-setup-ghc': 'true'})).toThrow()
+      expect(() =>
+        getOpts(def(os), os, {
+          'stack-no-global': 'true',
+          'cabal-version': 'latest'
+        })
+      ).toThrow()
     );
   });
 });
diff --git a/action.yml b/action.yml
@@ -1,6 +1,6 @@
 name: 'Setup Haskell'
 description: 'Set up a specific version of GHC and Cabal and add the command-line tools to the PATH'
-author: 'GitHub'
+author: 'Haskell community'
 inputs:
   ghc-version:
     required: false
@@ -16,13 +16,16 @@ inputs:
     default: 'latest'
   enable-stack:
     required: false
-    description: 'If specified, will setup Stack.'
+    default: false
+    description: 'If set to `true`, will setup default Stack.  Implied by any of `stack-version`, `stack-no-global`, `stack-setup-ghc`.'
   stack-no-global:
     required: false
-    description: 'If specified, enable-stack must be set. Prevents installing GHC and Cabal globally.'
+    default: false
+    description: 'If set to `true`, will setup Stack but will not install GHC and Cabal globally.'
   stack-setup-ghc:
     required: false
-    description: 'If specified, enable-stack must be set. Will run stack setup to install the specified GHC.'
+    default: false
+    description: 'If set to `true`, will setup Stack. Will run `stack setup` to install the specified GHC.'
   cabal-update:
     required: false
     default: true
@@ -33,9 +36,14 @@ inputs:
   ghcup-release-channel:
     required: false
     description: "A release channel URL to add to ghcup via `ghcup config add-release-channel`."
+  enable-matcher:
+    required: false
+    default: true
+    description: 'Enable match messages from GHC as GitHub CI annotations.'
   disable-matcher:
     required: false
-    description: 'If specified, disables match messages from GHC as GitHub CI annotations.'
+    default: false
+    description: 'Legacy input, use `enable-matcher` instead.'
 outputs:
   ghc-path:
     description: 'The path of the ghc executable _directory_'
diff --git a/dist/index.js b/dist/index.js
@@ -13714,7 +13714,8 @@ exports.ghcup_version = sv.ghcup[0]; // Known to be an array of length 1
  *   },
  *   'enable-stack': {
  *     required: false,
- *     default: 'latest'
+ *     description: '...',
+ *     default: false
  *   },
  *   ...
  * }
@@ -13777,8 +13778,39 @@ function parseYAMLBoolean(name, val) {
         `Supported boolean values: \`true | True | TRUE | false | False | FALSE\``);
 }
 exports.parseYAMLBoolean = parseYAMLBoolean;
+function parseBooleanInput(inputs, name, def) {
+    const val = inputs[name];
+    return val ? parseYAMLBoolean(name, val) : def;
+}
+/**
+ * Parse two opposite boolean options, one with default 'true' and the other with default 'false'.
+ * Return the value of the positive option.
+ * E.g. 'enable-matcher: true' and 'disable-matcher: false' would result in 'true'.
+ *
+ * @param inputs    options as key-value map
+ * @param positive  name (key) of the positive option (defaults to 'true')
+ * @param negative  name (key) of the negative option (defaults to 'false')
+ */
+function parseOppositeBooleanInputs(inputs, positive, negative) {
+    if (!inputs[negative]) {
+        return parseBooleanInput(inputs, positive, true);
+    }
+    else if (!inputs[positive]) {
+        return !parseBooleanInput(inputs, negative, false);
+    }
+    else {
+        const pos = parseBooleanInput(inputs, positive, true);
+        const neg = parseBooleanInput(inputs, negative, false);
+        if (pos == !neg) {
+            return pos;
+        }
+        else {
+            throw new Error(`Action input ${positive}: ${pos} contradicts ${negative}: ${neg}`);
+        }
+    }
+}
 function parseURL(name, val) {
-    if (val === '')
+    if (!val)
         return undefined;
     try {
         return new URL(val);
@@ -13788,36 +13820,49 @@ function parseURL(name, val) {
     }
 }
 exports.parseURL = parseURL;
+function parseURLInput(inputs, name) {
+    return parseURL(name, inputs[name]);
+}
 function getOpts({ ghc, cabal, stack }, os, inputs) {
     core.debug(`Inputs are: ${JSON.stringify(inputs)}`);
-    const stackNoGlobal = (inputs['stack-no-global'] || '') !== '';
-    const stackSetupGhc = (inputs['stack-setup-ghc'] || '') !== '';
-    const stackEnable = (inputs['enable-stack'] || '') !== '';
-    const matcherDisable = (inputs['disable-matcher'] || '') !== '';
-    const ghcupReleaseChannel = parseURL('ghcup-release-channel', inputs['ghcup-release-channel'] || '');
-    // Andreas, 2023-01-05, issue #29:
-    // 'cabal-update' has a default value, so we should get a proper boolean always.
-    // Andreas, 2023-01-06: This is not true if we use the action as a library.
-    // Thus, need to patch with default value here.
-    const cabalUpdate = parseYAMLBoolean('cabal-update', inputs['cabal-update'] || 'true');
-    core.debug(`${stackNoGlobal}/${stackSetupGhc}/${stackEnable}`);
+    const ghcVersion = inputs['ghc-version'];
+    const cabalVersion = inputs['cabal-version'];
+    const stackVersion = inputs['stack-version'];
+    const stackNoGlobal = parseBooleanInput(inputs, 'stack-no-global', false);
+    const stackSetupGhc = parseBooleanInput(inputs, 'stack-setup-ghc', false);
+    const stackDefault = stackNoGlobal || stackSetupGhc || !!stackVersion;
+    const stackEnable = parseBooleanInput(inputs, 'enable-stack', stackDefault);
+    const ghcEnable = !stackNoGlobal;
+    const cabalEnable = !stackNoGlobal;
+    const cabalUpdate = parseBooleanInput(inputs, 'cabal-update', cabalEnable);
+    const matcherEnable = parseOppositeBooleanInputs(inputs, 'enable-matcher', 'disable-matcher');
+    // disable-matcher is kept for backwards compatibility
+    // positive options like enable-matcher are preferable
+    const ghcupReleaseChannel = parseURLInput(inputs, 'ghcup-release-channel');
     const verInpt = {
-        ghc: inputs['ghc-version'] || ghc.version,
-        cabal: inputs['cabal-version'] || cabal.version,
-        stack: inputs['stack-version'] || stack.version
+        ghc: ghcVersion || ghc.version,
+        cabal: cabalVersion || cabal.version,
+        stack: stackVersion || stack.version
     };
+    // Check inputs for consistency
     const errors = [];
-    if (stackNoGlobal && !stackEnable) {
-        errors.push('enable-stack is required if stack-no-global is set');
-    }
-    if (stackSetupGhc && !stackEnable) {
-        errors.push('enable-stack is required if stack-setup-ghc is set');
+    if (!stackEnable) {
+        if (stackNoGlobal)
+            errors.push('Action input `enable-stack: false` contradicts `stack-no-global: true`');
+        if (stackSetupGhc)
+            errors.push('Action input `enable-stack: false` contradicts `stack-setup-ghc: true`');
+        if (stackVersion)
+            errors.push('Action input `enable-stack: false` contradicts setting `stack-version`');
+    }
+    if (stackNoGlobal) {
+        if (ghcVersion)
+            errors.push('Action input `stack-no-global: true` contradicts setting `ghc-version');
+        if (cabalVersion)
+            errors.push('Action input `stack-no-global: true` contradicts setting `cabal-version');
     }
     if (errors.length > 0) {
         throw new Error(errors.join('\n'));
     }
-    const ghcEnable = !stackNoGlobal;
-    const cabalEnable = !stackNoGlobal;
     const opts = {
         ghc: {
             raw: verInpt.ghc,
@@ -13842,7 +13887,7 @@ function getOpts({ ghc, cabal, stack }, os, inputs) {
             enable: stackEnable,
             setup: stackSetupGhc
         },
-        general: { matcher: { enable: !matcherDisable } }
+        general: { matcher: { enable: matcherEnable } }
     };
     core.debug(`Options are: ${JSON.stringify(opts)}`);
     return opts;
diff --git a/lib/opts.d.ts b/lib/opts.d.ts
@@ -63,7 +63,8 @@ export type Defaults = Record<Tool, Version> & {
  *   },
  *   'enable-stack': {
  *     required: false,
- *     default: 'latest'
+ *     description: '...',
+ *     default: false
  *   },
  *   ...
  * }
diff --git a/lib/opts.js b/lib/opts.js
diff --git a/src/opts.ts b/src/opts.ts
