Skip to content

Commit d7c083d

Browse files
authored
feat(jsii-diff): remap old to new FQNs (#4976)
Add a new feature: remapping of FQNs. This allows proper API compatiblity checking of `jsii-diff` even if types get moved around. Now normally you shouldn't move types around because it breaks backwards compatibility anyway, but in case you did and you want a handle not not accidentally changing other things, you can confirm it with this feature. --- By submitting this pull request, I confirm that my contribution is made under the terms of the [Apache 2.0 license]. [Apache 2.0 license]: https://www.apache.org/licenses/LICENSE-2.0
1 parent 86995e5 commit d7c083d

File tree

8 files changed

+420
-240
lines changed

8 files changed

+420
-240
lines changed

packages/jsii-diff/README.md

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -126,6 +126,45 @@ abstract members yet.
126126
for subclassing, but treating them as such would limit the evolvability of
127127
libraries too much.
128128

129+
## Accepting breaking changes
130+
131+
Sometimes you want to move forward with a change, even if it would technically
132+
be a breaking change to your API. In order to do that, run `jsii-diff`
133+
with the flag `--keys`. It will then print an identifier for every violation
134+
between square brackets, for example:
135+
136+
```
137+
$ jsii-diff <old> --keys
138+
IFACE pkg.MyStruct: formerly required property 'prop' is optional: returned from pkg.IConsumer.someMethod [weakened:pkg.MyStruct]
139+
```
140+
141+
To accept a breaking finding, put the key (in this example `weakened:pkg.MyStruct`)
142+
into a text file, for example `allowed-breaking-changes.txt`, and pass it to
143+
`jsii-diff` as an ignore file:
144+
145+
```
146+
$ jsii-diff <old> --keys --ignore-file allowed-breaking-changes.txt
147+
(no error)
148+
```
149+
150+
### Moving/renaming API elements
151+
152+
If you've moved API elements around between versions of your library, you can
153+
put a special ignore marker starting with `move:` into your `--ignore-file`. To
154+
separate the old and new class names, you can use `:`, `,` or whitespace.
155+
156+
For example:
157+
158+
```
159+
move:package.OldClassName package.NewClassName
160+
move:package.OldClassName:package.NewClassName
161+
move:package.OldClassName, package.NewClassName
162+
```
163+
164+
Moving API elements is always breaking, but using this feature you can confirm
165+
that you at least didn't break anything in the API surface of the moved classes
166+
themselves.
167+
129168
## Help! jsii-diff is marking my changes as breaking
130169

131170
See [BREAKING_CHANGES.md](./BREAKING_CHANGES.md) for more information.

packages/jsii-diff/bin/jsii-diff.ts

Lines changed: 45 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -124,9 +124,15 @@ async function main(): Promise<number> {
124124
);
125125
}
126126

127+
const allowedBreakingChanges = await loadFilter(argv['ignore-file']);
128+
const fqnRemapping: Record<string, string> = extractFqnRemappings(
129+
allowedBreakingChanges,
130+
);
131+
127132
LOG.info('Starting analysis');
128133
const mismatches = compareAssemblies(original, updated, {
129134
defaultExperimental: argv['default-stability'] === 'experimental',
135+
fqnRemapping,
130136
});
131137

132138
LOG.info(`Found ${mismatches.count} issues`);
@@ -135,7 +141,7 @@ async function main(): Promise<number> {
135141
const diags = classifyDiagnostics(
136142
mismatches,
137143
treatAsError(argv['error-on'] as ErrorClass, argv['experimental-errors']),
138-
await loadFilter(argv['ignore-file']),
144+
allowedBreakingChanges,
139145
);
140146

141147
process.stderr.write(
@@ -155,6 +161,43 @@ async function main(): Promise<number> {
155161
return 0;
156162
}
157163

164+
/**
165+
* Extract all lines that start with `move:` from the given string set
166+
*
167+
* Interpret them as `move:OLDFQN <sep> NEWFQN`, mapping moved FQNs.
168+
*
169+
* Separator can be any of `:`, comma or whitespace.
170+
*
171+
* Modifies the input set in-place.
172+
*/
173+
function extractFqnRemappings(
174+
allowedBreakingChanges: Set<string>,
175+
): Record<string, string> {
176+
const ret: Record<string, string> = {};
177+
178+
for (const line of Array.from(allowedBreakingChanges)) {
179+
const prefix = 'move:';
180+
if (!line.startsWith(prefix)) {
181+
continue;
182+
}
183+
184+
const parts = line
185+
.slice(prefix.length)
186+
.trim()
187+
.split(/[:, \t]+/g);
188+
if (parts.length !== 2) {
189+
throw new Error(
190+
`Invalid moved FQN declaration: ${line}. Expected format is 'move:old:new'`,
191+
);
192+
}
193+
const [oldFqn, newFqn] = parts;
194+
ret[oldFqn] = newFqn;
195+
allowedBreakingChanges.delete(line);
196+
}
197+
198+
return ret;
199+
}
200+
158201
// Allow both npm:<package> (legacy) and npm://<package> (looks better)
159202
const NPM_REGEX = /^npm:(\/\/)?/;
160203

@@ -311,7 +354,7 @@ async function loadFilter(filterFilename?: string): Promise<Set<string>> {
311354
(await fs.readFile(filterFilename, { encoding: 'utf-8' }))
312355
.split('\n')
313356
.map((x) => x.trim())
314-
.filter((x) => !x.startsWith('#')),
357+
.filter((x) => x && !x.startsWith('#')),
315358
);
316359
} catch (e: any) {
317360
if (e.code !== 'ENOENT') {

0 commit comments

Comments
 (0)