Skip to content

Commit 33d1d6d

Browse files
committed
Integrate the new parser and update tests
1 parent b936033 commit 33d1d6d

File tree

2 files changed

+134
-102
lines changed

2 files changed

+134
-102
lines changed

src/analyzer.ts

Lines changed: 116 additions & 85 deletions
Original file line numberDiff line numberDiff line change
@@ -14,14 +14,20 @@ import {
1414
} from './util/package-json-reader.js';
1515
import {IS_WINDOWS} from './util/windows.js';
1616

17+
import {parseDependency} from './analysis/dependency-parser.js';
1718
import type {Agent} from './cli-options.js';
1819
import type {
1920
ScriptConfig,
2021
ScriptReference,
2122
ScriptReferenceString,
2223
} from './config.js';
23-
import type {Diagnostic, MessageLocation, Result} from './error.js';
24-
import type {Cycle, DependencyOnMissingPackageJson, Failure} from './event.js';
24+
import type {Diagnostic, MessageLocation, Range, Result} from './error.js';
25+
import type {
26+
Cycle,
27+
DependencyOnMissingPackageJson,
28+
Failure,
29+
InvalidConfigSyntax,
30+
} from './event.js';
2531
import {Logger} from './logging/logger.js';
2632
import type {
2733
ArrayNode,
@@ -1708,109 +1714,115 @@ export class Analyzer {
17081714
context: ScriptReference,
17091715
referencingFile: JsonFile,
17101716
): Result<Array<ScriptReference>, Failure> {
1711-
// TODO(aomarks) Implement $WORKSPACES syntax.
1712-
if (dependency.value.startsWith('.')) {
1713-
// TODO(aomarks) It is technically valid for an npm script to start with a
1714-
// ".". We should support that edge case with backslash escaping.
1715-
const result = this.#resolveCrossPackageDependency(
1716-
dependency,
1717-
context,
1718-
referencingFile,
1719-
);
1720-
if (!result.ok) {
1721-
return result;
1722-
}
1723-
return {ok: true, value: [result.value]};
1724-
}
1725-
return {
1726-
ok: true,
1727-
value: [{packageDir: context.packageDir, name: dependency.value}],
1728-
};
1729-
}
1730-
1731-
/**
1732-
* Resolve a cross-package dependency (e.g. "../other-package:build").
1733-
* Cross-package dependencies always start with a ".".
1734-
*/
1735-
#resolveCrossPackageDependency(
1736-
dependency: JsonAstNode<string>,
1737-
context: ScriptReference,
1738-
referencingFile: JsonFile,
1739-
): Result<ScriptReference, Failure> {
1740-
// TODO(aomarks) On some file systems, it is valid to have a ":" in a file
1741-
// path. We should support that edge case with backslash escaping.
1742-
const firstColonIdx = dependency.value.indexOf(':');
1743-
if (firstColonIdx === -1) {
1717+
const parsed = parseDependency(dependency.value);
1718+
if (!parsed.ok) {
17441719
return {
17451720
ok: false,
17461721
error: {
17471722
type: 'failure',
17481723
reason: 'invalid-config-syntax',
17491724
script: context,
17501725
diagnostic: {
1751-
severity: 'error',
1752-
message:
1753-
`Cross-package dependency must use syntax ` +
1754-
`"<relative-path>:<script-name>", ` +
1755-
`but there's no ":" character in "${dependency.value}".`,
1726+
...parsed.error,
17561727
location: {
1728+
// The parser doesn't know about the file, add that to the
1729+
// diagnostic.
17571730
file: referencingFile,
1758-
range: {offset: dependency.offset, length: dependency.length},
1731+
range: {
1732+
offset:
1733+
dependency.offset + 1 + parsed.error.location.range.offset,
1734+
length: parsed.error.location.range.length,
1735+
},
17591736
},
17601737
},
17611738
},
17621739
};
17631740
}
1764-
const scriptName = dependency.value.slice(firstColonIdx + 1);
1765-
if (!scriptName) {
1766-
return {
1767-
ok: false,
1768-
error: {
1769-
type: 'failure',
1770-
reason: 'invalid-config-syntax',
1771-
script: context,
1772-
diagnostic: {
1773-
severity: 'error',
1774-
message:
1775-
`Cross-package dependency must use syntax ` +
1776-
`"<relative-path>:<script-name>", ` +
1777-
`but there's no script name in "${dependency.value}".`,
1778-
location: {
1779-
file: referencingFile,
1780-
range: {offset: dependency.offset, length: dependency.length},
1781-
},
1782-
},
1783-
},
1784-
};
1741+
1742+
const {package: pkg, script, inverted} = parsed.value;
1743+
1744+
if (inverted) {
1745+
// TODO(aomarks) Support inversion.
1746+
return invalidSyntaxError(
1747+
'Dependency inversion operator "!" is not yet supported',
1748+
context,
1749+
referencingFile,
1750+
{offset: dependency.offset, length: 1},
1751+
);
17851752
}
1786-
const relativePackageDir = dependency.value.slice(0, firstColonIdx);
1787-
const absolutePackageDir = pathlib.resolve(
1788-
context.packageDir,
1789-
relativePackageDir,
1790-
);
1791-
if (absolutePackageDir === context.packageDir) {
1792-
return {
1793-
ok: false,
1794-
error: {
1795-
type: 'failure',
1796-
reason: 'invalid-config-syntax',
1797-
script: context,
1798-
diagnostic: {
1799-
severity: 'error',
1800-
message:
1801-
`Cross-package dependency "${dependency.value}" ` +
1802-
`resolved to the same package.`,
1803-
location: {
1804-
file: referencingFile,
1805-
range: {offset: dependency.offset, length: dependency.length},
1806-
},
1753+
1754+
let packageDir: string;
1755+
if (pkg.kind === 'this') {
1756+
packageDir = context.packageDir;
1757+
} else if (pkg.kind === 'path') {
1758+
packageDir = pathlib.resolve(context.packageDir, pkg.path);
1759+
if (packageDir === context.packageDir) {
1760+
return invalidSyntaxError(
1761+
`Cross-package dependency "${dependency.value}" ` +
1762+
`resolved to the same package.`,
1763+
context,
1764+
referencingFile,
1765+
{
1766+
offset: dependency.offset + 1 + pkg.range.offset,
1767+
length: pkg.range.length,
18071768
},
1769+
);
1770+
}
1771+
} else if (pkg.kind === 'npm') {
1772+
// TODO(aomarks) Support npm resolution.
1773+
return invalidSyntaxError(
1774+
'NPM packages are not yet supported',
1775+
context,
1776+
referencingFile,
1777+
{
1778+
offset: dependency.offset + 1 + pkg.range.offset,
1779+
length: pkg.range.length,
18081780
},
1809-
};
1781+
);
1782+
} else if (pkg.kind === 'dependencies') {
1783+
// TODO(aomarks) Support "dependencies".
1784+
return invalidSyntaxError(
1785+
`"dependencies" is not yet supported`,
1786+
context,
1787+
referencingFile,
1788+
{
1789+
offset: dependency.offset + 1 + pkg.range.offset,
1790+
length: pkg.range.length,
1791+
},
1792+
);
1793+
} else if (pkg.kind === 'workspaces') {
1794+
// TODO(aomarks) Support "workspacess".
1795+
return invalidSyntaxError(
1796+
`"workspaces" is not yet supported`,
1797+
context,
1798+
referencingFile,
1799+
{
1800+
offset: dependency.offset + 1 + pkg.range.offset,
1801+
length: pkg.range.length,
1802+
},
1803+
);
1804+
} else {
1805+
pkg satisfies never;
1806+
throw new Error(
1807+
`Unexpected parsed package format: ${JSON.stringify(pkg)}`,
1808+
);
1809+
}
1810+
1811+
let scriptName: string;
1812+
if (script.kind === 'name') {
1813+
scriptName = script.name;
1814+
} else if (script.kind === 'this') {
1815+
scriptName = context.name;
1816+
} else {
1817+
script satisfies never;
1818+
throw new Error(
1819+
`Unexpected parsed script format: ${JSON.stringify(script)}`,
1820+
);
18101821
}
1822+
18111823
return {
18121824
ok: true,
1813-
value: {packageDir: absolutePackageDir, name: scriptName},
1825+
value: [{packageDir, name: scriptName}],
18141826
};
18151827
}
18161828
}
@@ -1967,3 +1979,22 @@ export const failUnlessKeyValue = (
19671979
}
19681980
return {ok: true, value: [rawName, rawValue]};
19691981
};
1982+
1983+
const invalidSyntaxError = (
1984+
message: string,
1985+
script: ScriptReference,
1986+
file: JsonFile,
1987+
range: Range,
1988+
): {ok: false; error: InvalidConfigSyntax} => ({
1989+
ok: false,
1990+
error: {
1991+
type: 'failure',
1992+
reason: 'invalid-config-syntax',
1993+
script,
1994+
diagnostic: {
1995+
message,
1996+
severity: 'error',
1997+
location: {file, range},
1998+
},
1999+
},
2000+
});

src/test/errors-analysis.test.ts

Lines changed: 18 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -4,12 +4,12 @@
44
* SPDX-License-Identifier: Apache-2.0
55
*/
66

7+
import * as pathlib from 'path';
78
import {suite} from 'uvu';
89
import * as assert from 'uvu/assert';
9-
import * as pathlib from 'path';
10-
import {rigTest} from './util/rig-test.js';
1110
import {IS_WINDOWS} from '../util/windows.js';
1211
import {checkScriptOutput} from './util/check-script-output.js';
12+
import {rigTest} from './util/rig-test.js';
1313

1414
const test = suite<object>();
1515

@@ -774,6 +774,7 @@ test(
774774
);
775775
}),
776776
);
777+
777778
test(
778779
'missing cross package dependency with complicated escaped names',
779780
rigTest(async ({rig}) => {
@@ -788,7 +789,7 @@ test(
788789
},
789790
wireit: {
790791
a: {
791-
dependencies: ['./ch\t\\ ild:mis\t\\ sing'],
792+
dependencies: ['./ch\t\\\\ ild:mis\t\\\\ sing'],
792793
},
793794
},
794795
},
@@ -802,11 +803,11 @@ test(
802803
checkScriptOutput(
803804
done.stderr,
804805
String.raw`
805-
❌ package.json:8:23 Cannot find script named "mis\t\\ sing" in package "${rig.resolve(
806+
❌ package.json:8:25 Cannot find script named "mis\t\\ sing" in package "${rig.resolve(
806807
'ch\t\\ ild',
807808
)}"
808-
"./ch\t\\ ild:mis\t\\ sing"
809-
~~~~~~~~~~~~`,
809+
"./ch\t\\\\ ild:mis\t\\\\ sing"
810+
~~~~~~~~~~~~~~`,
810811
);
811812
}),
812813
);
@@ -821,7 +822,7 @@ test(
821822
},
822823
wireit: {
823824
a: {
824-
dependencies: ['../b\t\\ ar:b'],
825+
dependencies: ['../b\t\\\\ ar:b'],
825826
},
826827
},
827828
},
@@ -835,8 +836,8 @@ test(
835836
❌ package.json:8:10 package.json file missing: "${rig.resolve(
836837
'b\t\\ ar/package.json',
837838
)}"
838-
"../b\t\\ ar:b"
839-
~~~~~~~~~~~`,
839+
"../b\t\\\\ ar:b"
840+
~~~~~~~~~~~~~`,
840841
);
841842
}),
842843
);
@@ -1012,9 +1013,9 @@ test(
10121013
checkScriptOutput(
10131014
done.stderr,
10141015
`
1015-
❌ package.json:8:9 Cross-package dependency must use syntax "<relative-path>:<script-name>", but there's no ":" character in "../foo".
1016+
❌ package.json:8:10 Cross-package dependency must use syntax "<relative-path>#<script-name>", but there's no "#" character in "../foo".
10161017
"../foo"
1017-
~~~~~~~~
1018+
~~~~~~
10181019
`,
10191020
);
10201021
}),
@@ -1041,9 +1042,9 @@ test(
10411042
checkScriptOutput(
10421043
done.stderr,
10431044
`
1044-
❌ package.json:8:9 Cross-package dependency must use syntax "<relative-path>:<script-name>", but there's no script name in "../foo:".
1045+
❌ package.json:8:10 Cross-package dependency must use syntax "<relative-path>#<script-name>", but there's no script name in "../foo:".
10451046
"../foo:"
1046-
~~~~~~~~~
1047+
~~~~~~~
10471048
`,
10481049
);
10491050
}),
@@ -1070,9 +1071,9 @@ test(
10701071
checkScriptOutput(
10711072
done.stderr,
10721073
`
1073-
❌ package.json:8:9 Cross-package dependency ".:b" resolved to the same package.
1074+
❌ package.json:8:10 Cross-package dependency ".:b" resolved to the same package.
10741075
".:b"
1075-
~~~~~
1076+
~
10761077
`,
10771078
);
10781079
}),
@@ -1099,9 +1100,9 @@ test(
10991100
checkScriptOutput(
11001101
done.stderr,
11011102
`
1102-
❌ package.json:8:9 Cross-package dependency "../foo:b" resolved to the same package.
1103+
❌ package.json:8:10 Cross-package dependency "../foo:b" resolved to the same package.
11031104
"../foo:b"
1104-
~~~~~~~~~~
1105+
~~~~~~
11051106
`,
11061107
);
11071108
}),

0 commit comments

Comments
 (0)