Allow wireit-only scripts

CHANGELOG.md

@@ -6,7 +6,14 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
 and this project adheres to [Semantic
 Versioning](https://semver.org/spec/v2.0.0.html).
 
-<!-- ## [Unreleased] -->
+## [Unreleased]
+
+### Changed
+
+- It is now allowed to define a wireit script without a corresponding entry in
+  the `scripts` section. Such scripts cannot be directly invoked with `npm run
+  <script>` or similar, but they can still be used as dependencies by other
+  wireit scripts.
 
 ## [0.9.3] - 2023-01-03
 

README.md

@@ -161,6 +161,12 @@ vanilla `npm` scripts. This lets you only use Wireit for some of your scripts,
 or to upgrade incrementally. Scripts that haven't been configured for Wireit are
 always safe to use as dependencies; they just won't be fully optimized.
 
+### Wireit-only scripts
+
+It is valid to define a script in the `wireit` section that is not in the
+`scripts` section, but such scripts can only be used as `dependencies` from
+other wireit scripts, and can never be run directly.
+
 ### Cross-package dependencies
 
 Dependencies can refer to scripts in other npm packages by using a relative path

src/analyzer.ts

@@ -14,7 +14,12 @@ import {Dependency, scriptReferenceToString, ServiceConfig} from './config.js';
 import {findNodeAtLocation, JsonFile} from './util/ast.js';
 import {IS_WINDOWS} from './util/windows.js';
 
-import type {ArrayNode, JsonAstNode, NamedAstNode} from './util/ast.js';
+import type {
+  ArrayNode,
+  JsonAstNode,
+  NamedAstNode,
+  ValueTypes,
+} from './util/ast.js';
 import type {Diagnostic, MessageLocation, Result} from './error.js';
 import type {Cycle, DependencyOnMissingPackageJson, Failure} from './event.js';
 import type {PackageJson, ScriptSyntaxInfo} from './util/package-json.js';
@@ -117,6 +122,18 @@ const DEFAULT_LOCKFILES: Record<Agent, string[]> = {
   pnpm: ['pnpm-lock.yaml'],
 };
 
+function isValidWireitScriptCommand(command: string): boolean {
+  return (
+    command === 'wireit' ||
+    command === 'yarn run -TB wireit' ||
+    // This form is useful when using package managers like yarn or pnpm which
+    // do not automatically add all parent directory `node_modules/.bin`
+    // folders to PATH.
+    /^(\.\.\/)+node_modules\/\.bin\/wireit$/.test(command) ||
+    (IS_WINDOWS && /^(\.\.\\)+node_modules\\\.bin\\wireit\.cmd$/.test(command))
+  );
+}
+
 /**
  * Analyzes and validates a script along with all of its transitive
  * dependencies, producing a build graph that is ready to be executed.
@@ -344,79 +361,43 @@ export class Analyzer {
     placeholder.failures.push(...packageJson.failures);
 
     const syntaxInfo = packageJson.getScriptInfo(placeholder.name);
-    if (syntaxInfo === undefined || syntaxInfo.scriptNode === undefined) {
-      let node;
-      let reason;
-      if (syntaxInfo?.wireitConfigNode?.name !== undefined) {
-        node = syntaxInfo.wireitConfigNode.name;
-        reason = 'wireit-config-but-no-script' as const;
-      } else {
-        node ??= packageJson.scriptsSection?.name;
-        reason = 'script-not-found' as const;
-      }
-      const range = node
-        ? {offset: node.offset, length: node.length}
-        : {offset: 0, length: 0};
+    if (syntaxInfo?.wireitConfigNode !== undefined) {
+      await this._handleWireitScript(
+        placeholder,
+        packageJson,
+        syntaxInfo,
+        syntaxInfo.wireitConfigNode
+      );
+    } else if (syntaxInfo?.scriptNode !== undefined) {
+      this._handlePlainNpmScript(
+        placeholder,
+        packageJson,
+        syntaxInfo.scriptNode
+      );
+    } else {
       placeholder.failures.push({
         type: 'failure',
-        reason,
+        reason: 'script-not-found',
         script: placeholder,
         diagnostic: {
           severity: 'error',
           message: `Script "${placeholder.name}" not found in the scripts section of this package.json.`,
-          location: {file: packageJson.jsonFile, range},
-        },
-      });
-      return undefined;
-    }
-    const scriptCommand = syntaxInfo.scriptNode;
-    const wireitConfig = syntaxInfo.wireitConfigNode;
-
-    if (
-      wireitConfig !== undefined &&
-      scriptCommand.value !== 'wireit' &&
-      scriptCommand.value !== 'yarn run -TB wireit' &&
-      // This form is useful when using package managers like yarn or pnpm which
-      // do not automatically add all parent directory `node_modules/.bin`
-      // folders to PATH.
-      !/^(\.\.\/)+node_modules\/\.bin\/wireit$/.test(scriptCommand.value) &&
-      !(
-        IS_WINDOWS &&
-        /^(\.\.\\)+node_modules\\\.bin\\wireit\.cmd$/.test(scriptCommand.value)
-      )
-    ) {
-      const configName = wireitConfig.name;
-      placeholder.failures.push({
-        type: 'failure',
-        reason: 'script-not-wireit',
-        script: placeholder,
-        diagnostic: {
-          message: `This command should just be "wireit", as this script is configured in the wireit section.`,
-          severity: 'warning',
           location: {
             file: packageJson.jsonFile,
-            range: {
-              length: scriptCommand.length,
-              offset: scriptCommand.offset,
-            },
+            range: {offset: 0, length: 0},
           },
-          supplementalLocations: [
-            {
-              message: `The wireit config is here.`,
-              location: {
-                file: packageJson.jsonFile,
-                range: {
-                  length: configName.length,
-                  offset: configName.offset,
-                },
-              },
-            },
-          ],
         },
       });
     }
+    return undefined;
+  }
 
-    if (wireitConfig === undefined && scriptCommand.value === 'wireit') {
+  private _handlePlainNpmScript(
+    placeholder: UnvalidatedConfig,
+    packageJson: PackageJson,
+    scriptCommand: NamedAstNode<string>
+  ): void {
+    if (isValidWireitScriptCommand(scriptCommand.value)) {
       placeholder.failures.push({
         type: 'failure',
         reason: 'invalid-config-syntax',
@@ -434,39 +415,91 @@ export class Analyzer {
         },
       });
     }
+    // It's important to in-place update the placeholder object, instead of
+    // creating a new object, because other configs may be referencing this
+    // exact object in their dependencies.
+    const remainingConfig: LocallyValidScriptConfig = {
+      ...placeholder,
+      state: 'locally-valid',
+      failures: placeholder.failures,
+      command: scriptCommand,
+      extraArgs: undefined,
+      dependencies: [],
+      files: undefined,
+      output: undefined,
+      clean: false,
+      service: undefined,
+      scriptAstNode: scriptCommand,
+      configAstNode: undefined,
+      declaringFile: packageJson.jsonFile,
+      services: [],
+      env: {},
+    };
+    Object.assign(placeholder, remainingConfig);
+  }
+
+  private async _handleWireitScript(
+    placeholder: UnvalidatedConfig,
+    packageJson: PackageJson,
+    syntaxInfo: ScriptSyntaxInfo,
+    wireitConfig: NamedAstNode<ValueTypes>
+  ): Promise<void> {
+    const scriptCommand = syntaxInfo.scriptNode;
+    if (
+      scriptCommand !== undefined &&
+      !isValidWireitScriptCommand(scriptCommand.value)
+    ) {
+      {
+        const configName = wireitConfig.name;
+        placeholder.failures.push({
+          type: 'failure',
+          reason: 'script-not-wireit',
+          script: placeholder,
+          diagnostic: {
+            message:
+              `This command should just be "wireit", ` +
+              `as this script is configured in the wireit section.`,
+            severity: 'warning',
+            location: {
+              file: packageJson.jsonFile,
+              range: {
+                length: scriptCommand.length,
+                offset: scriptCommand.offset,
+              },
+            },
+            supplementalLocations: [
+              {
+                message: `The wireit config is here.`,
+                location: {
+                  file: packageJson.jsonFile,
+                  range: {
+                    length: configName.length,
+                    offset: configName.offset,
+                  },
+                },
+              },
+            ],
+          },
+        });
+      }
+    }
 
     const {dependencies, encounteredError: dependenciesErrored} =
       this._processDependencies(placeholder, packageJson, syntaxInfo);
 
     let command: JsonAstNode<string> | undefined;
     let commandError = false;
-    if (wireitConfig === undefined) {
-      const result = failUnlessNonBlankString(
-        scriptCommand,
-        packageJson.jsonFile
-      );
+    const commandAst = findNodeAtLocation(wireitConfig, ['command']) as
+      | undefined
+      | JsonAstNode<string>;
+    if (commandAst !== undefined) {
+      const result = failUnlessNonBlankString(commandAst, packageJson.jsonFile);
       if (result.ok) {
         command = result.value;
       } else {
         commandError = true;
         placeholder.failures.push(result.error);
       }
-    } else {
-      const commandAst = findNodeAtLocation(wireitConfig, ['command']) as
-        | undefined
-        | JsonAstNode<string>;
-      if (commandAst !== undefined) {
-        const result = failUnlessNonBlankString(
-          commandAst,
-          packageJson.jsonFile
-        );
-        if (result.ok) {
-          command = result.value;
-        } else {
-          commandError = true;
-          placeholder.failures.push(result.error);
-        }
-      }
     }
 
     const allowUsuallyExcludedPaths = this._processAllowUsuallyExcludedPaths(
@@ -483,7 +516,6 @@ export class Analyzer {
     );
 
     if (
-      wireitConfig !== undefined &&
       dependencies.length === 0 &&
       !dependenciesErrored &&
       command === undefined &&
@@ -1392,7 +1424,7 @@ export class Analyzer {
           nextNode?.specifier ??
           // But failing that, fall back to the best node we have.
           current.configAstNode?.name ??
-          current.scriptAstNode?.name;
+          current.scriptAstNode!.name;
         supplementalLocations.push({
           message,
           location: {
@@ -1414,10 +1446,10 @@ export class Analyzer {
           range: {
             length:
               config.configAstNode?.name.length ??
-              config.scriptAstNode?.name.length,
+              config.scriptAstNode!.name.length,
             offset:
               config.configAstNode?.name.offset ??
-              config.scriptAstNode?.name.length,
+              config.scriptAstNode!.name.length,
           },
         },
         supplementalLocations,

src/config.ts

@@ -193,7 +193,7 @@ interface BaseScriptConfig extends ScriptReference {
    *   }
    * ```
    */
-  scriptAstNode: NamedAstNode<string>;
+  scriptAstNode: NamedAstNode<string> | undefined;
 
   /**
    * The entire config in the wireit section. i.e.:

src/test/basic.test.ts

@@ -1232,4 +1232,38 @@ test(
   })
 );
 
+test(
+  'dependency which is not in script section',
+  timeout(async ({rig}) => {
+    const cmdA = await rig.newCommand();
+    const cmdB = await rig.newCommand();
+    await rig.write({
+      'package.json': {
+        scripts: {
+          a: 'wireit',
+        },
+        wireit: {
+          a: {
+            command: cmdA.command,
+            files: [],
+            output: [],
+            dependencies: ['b'],
+          },
+          b: {
+            command: cmdB.command,
+            files: [],
+            output: [],
+          },
+        },
+      },
+    });
+
+    const wireit = rig.exec('npm run a');
+    (await cmdB.nextInvocation()).exit(0);
+    (await cmdA.nextInvocation()).exit(0);
+    const {code} = await wireit.exit;
+    assert.equal(code, 0);
+  })
+);
+
 test.run();

src/test/errors-analysis.test.ts

@@ -93,37 +93,6 @@ test(
   })
 );
 
-test(
-  'wireit config but no entry in scripts section',
-  timeout(async ({rig}) => {
-    await rig.write({
-      'package.json': {
-        scripts: {
-          a: 'wireit',
-        },
-        wireit: {
-          a: {
-            dependencies: ['b'],
-          },
-          b: {
-            dependencies: [],
-          },
-        },
-      },
-    });
-    const result = rig.exec('npm run a');
-    const done = await result.exit;
-    assert.equal(done.code, 1);
-    checkScriptOutput(
-      done.stderr,
-      `
-❌ package.json:11:5 Script "b" not found in the scripts section of this package.json.
-        "b": {
-        ~~~`
-    );
-  })
-);
-
 test(
   'dependencies is not an array',
   timeout(async ({rig}) => {