Skip to content

Commit

Permalink
Don't upgrade a script to locally-valid if it has errors
Browse files Browse the repository at this point in the history
This way later scripts can assume locally-valid means no failures in the script that are locally determinable.

I think this was the original intent, and some code seems to assume this. See e.g. #803
  • Loading branch information
rictic committed Sep 4, 2023
1 parent edb7f28 commit f3f69cb
Show file tree
Hide file tree
Showing 2 changed files with 37 additions and 0 deletions.
6 changes: 6 additions & 0 deletions src/analyzer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -585,6 +585,12 @@ export class Analyzer {

const env = this.#processEnv(placeholder, packageJson, syntaxInfo, command);

if (placeholder.failures.length > 0) {
// A script with locally-determined errors doesn't get upgraded to
// locally-valid.
return;
}

// 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.
Expand Down
31 changes: 31 additions & 0 deletions src/test/ide.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -154,6 +154,37 @@ test('we can get cyclic dependency errors', async ({rig}) => {
});
});

test('warns for a service without a command', async ({rig}) => {
const ide = new IdeAnalyzer();
ide.setOpenFileContents(
rig.resolve('package.json'),
JSON.stringify(
{
scripts: {
a: 'wireit',
b: 'wireit',
},
wireit: {
a: {
service: true,
dependencies: ['b'],
},
b: {
command: 'echo',
},
},
},
null,
2,
),
);
await assertDiagnostics(ide, {
[rig.resolve('package.json')]: [
`A "service" script must have a "command".`,
],
});
});

async function assertDefinition(
ide: IdeAnalyzer,
options: {
Expand Down

0 comments on commit f3f69cb

Please sign in to comment.