Skip to content

Commit 51211a0

Browse files
rix0rrrElad Ben-Israel
rix0rrr
authored and
Elad Ben-Israel
committed
feat(python): idiomatic capitalization for structs (#586)
This change addresses 4 issues: - Structs now use idiomatic (snake_case) capitalization of fields (instead of JSII-inherited camelCase). - IDE support -- replace TypedDict usage with regular classes. This makes it so that we can't use dicts anymore, but mypy support in IDEs wasn't great and by using POPOs (Plain Old Python Objects) IDEs get their support back. - Structs in a variadic argument use to be incorrectly lifted to keyword arguments, this no longer happens. - Stop emitting "Stable" stabilities in docstrings, "Stable" is implied. In order to make this change, I've had to make `jsii-pacmak` depend on `jsii-reflect`. This is the proper layering of libraries anyway, since `jsii-reflect` adds much-needed--and otherwise duplicated--interpretation of the spec. Complete refactoring of `jsii-pacmak` to build on `jsii-reflect` is too much work right now, however, so I've added "escape hatching" where generators can take advantage of the power of jsii-reflect if they want to, but most of the code still works on the raw spec level. Added a refactoring where we load the assembly once and reuse the same instance for all generators, instead of loading the assembly for every generator. Assembly-loading, especially with a lot of dependencies, takes a non-negligible amount of time, so this has the side effect of making the packaging step faster (shaves off 100 packages * 3 targets * a couple of seconds). Fixes #537 Fixes #577 Fixes #578 Fixes #588
1 parent c9fdecc commit 51211a0

File tree

43 files changed

+3898
-2143
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

43 files changed

+3898
-2143
lines changed

‎packages/codemaker/lib/case-utils.ts‎

Lines changed: 19 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,11 @@
11
import { default as camelcase } from 'camelcase';
2-
import * as decamelize from 'decamelize';
2+
import decamelize = require('decamelize');
3+
4+
const COMMON_ABBREVIATIONS = [
5+
'KiB',
6+
'MiB',
7+
'GiB',
8+
];
39

410
export function toCamelCase(...args: string[]) {
511
return camelcase(args);
@@ -10,5 +16,17 @@ export function toPascalCase(...args: string[]) {
1016
}
1117

1218
export function toSnakeCase(s: string, sep = '_') {
19+
// Save common abbrevations
20+
s = s.replace(ABBREV_RE, (_, before, abbr, after) => before + ucfirst(abbr.toLowerCase()) + after);
1321
return decamelize(s, sep);
1422
}
23+
24+
function regexQuote(s: string) {
25+
return s.replace(/[.?*+^$[\]\\(){}|-]/g, "\\$&");
26+
}
27+
28+
const ABBREV_RE = new RegExp('(^|[^A-Z])(' + COMMON_ABBREVIATIONS.map(regexQuote).join('|') + ')($|[^a-z])', 'g');
29+
30+
function ucfirst(s: string) {
31+
return s.substr(0, 1).toUpperCase() + s.substr(1).toLowerCase();
32+
}

‎packages/codemaker/test/test.case-utils.ts‎

Lines changed: 19 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,15 +2,32 @@ import nodeunit = require('nodeunit');
22
import caseUtils = require('../lib/case-utils');
33

44
export = nodeunit.testCase({
5-
toCamelCase(test: nodeunit.Test) {
5+
'toCamelCase'(test: nodeunit.Test) {
66
test.equal(caseUtils.toCamelCase('EXAMPLE_VALUE'), 'exampleValue');
77
test.equal(caseUtils.toCamelCase('example', 'value'), 'exampleValue');
88
test.done();
99
},
1010

11-
toPascalCase(test: nodeunit.Test) {
11+
'toPascalCase'(test: nodeunit.Test) {
1212
test.equal(caseUtils.toPascalCase('EXAMPLE_VALUE'), 'ExampleValue');
1313
test.equal(caseUtils.toPascalCase('example', 'value'), 'ExampleValue');
14+
test.done();
15+
},
16+
17+
'toSnakeCase'(test: nodeunit.Test) {
18+
test.equal(caseUtils.toSnakeCase('EXAMPLE_VALUE'), 'example_value');
19+
test.equal(caseUtils.toSnakeCase('exampleValue'), 'example_value');
20+
test.equal(caseUtils.toSnakeCase('ExampleValue'), 'example_value');
21+
test.equal(caseUtils.toSnakeCase('EPSConduit'), 'eps_conduit');
22+
test.equal(caseUtils.toSnakeCase('SomeEBSVolume'), 'some_ebs_volume');
23+
test.done();
24+
},
25+
26+
'reserved word snake-casing'(test: nodeunit.Test) {
27+
test.equal(caseUtils.toSnakeCase('SizeMiB'), 'size_mib');
28+
test.equal(caseUtils.toSnakeCase('SizeMiBiBytes'), 'size_mi_bi_bytes');
29+
test.equal(caseUtils.toSnakeCase('MiBSize'), 'mib_size');
30+
1431
test.done();
1532
}
1633
});

‎packages/jsii-calc/lib/compliance.ts‎

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1741,3 +1741,46 @@ export class DataRenderer {
17411741
return JSON.stringify(map, null, 2);
17421742
}
17431743
}
1744+
1745+
export interface TopLevelStruct {
1746+
/**
1747+
* This is a required field
1748+
*/
1749+
readonly required: string;
1750+
1751+
/**
1752+
* You don't have to pass this
1753+
*/
1754+
readonly optional?: string;
1755+
1756+
/**
1757+
* A union to really stress test our serialization
1758+
*/
1759+
readonly secondLevel: SecondLevelStruct | number;
1760+
}
1761+
1762+
export interface SecondLevelStruct {
1763+
/**
1764+
* It's long and required
1765+
*/
1766+
readonly deeperRequiredProp: string;
1767+
1768+
/**
1769+
* It's long, but you'll almost never pass it.
1770+
*/
1771+
readonly deeperOptionalProp?: string;
1772+
}
1773+
1774+
export class StructPassing {
1775+
public static roundTrip(_positional: number, input: TopLevelStruct): TopLevelStruct {
1776+
return {
1777+
required: input.required,
1778+
optional: input.optional,
1779+
secondLevel: input.secondLevel,
1780+
};
1781+
}
1782+
1783+
public static howManyVarArgsDidIPass(_positional: number, inputs: TopLevelStruct[]): number {
1784+
return inputs.length;
1785+
}
1786+
}

‎packages/jsii-calc/test/assembly.jsii‎

Lines changed: 205 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7150,6 +7150,55 @@
71507150
],
71517151
"name": "RuntimeTypeChecking"
71527152
},
7153+
"jsii-calc.SecondLevelStruct": {
7154+
"assembly": "jsii-calc",
7155+
"datatype": true,
7156+
"docs": {
7157+
"stability": "experimental"
7158+
},
7159+
"fqn": "jsii-calc.SecondLevelStruct",
7160+
"kind": "interface",
7161+
"locationInModule": {
7162+
"filename": "lib/compliance.ts",
7163+
"line": 1762
7164+
},
7165+
"name": "SecondLevelStruct",
7166+
"properties": [
7167+
{
7168+
"abstract": true,
7169+
"docs": {
7170+
"stability": "experimental",
7171+
"summary": "It's long and required."
7172+
},
7173+
"immutable": true,
7174+
"locationInModule": {
7175+
"filename": "lib/compliance.ts",
7176+
"line": 1766
7177+
},
7178+
"name": "deeperRequiredProp",
7179+
"type": {
7180+
"primitive": "string"
7181+
}
7182+
},
7183+
{
7184+
"abstract": true,
7185+
"docs": {
7186+
"stability": "experimental",
7187+
"summary": "It's long, but you'll almost never pass it."
7188+
},
7189+
"immutable": true,
7190+
"locationInModule": {
7191+
"filename": "lib/compliance.ts",
7192+
"line": 1771
7193+
},
7194+
"name": "deeperOptionalProp",
7195+
"optional": true,
7196+
"type": {
7197+
"primitive": "string"
7198+
}
7199+
}
7200+
]
7201+
},
71537202
"jsii-calc.SingleInstanceTwoTypes": {
71547203
"assembly": "jsii-calc",
71557204
"docs": {
@@ -7756,6 +7805,87 @@
77567805
}
77577806
]
77587807
},
7808+
"jsii-calc.StructPassing": {
7809+
"assembly": "jsii-calc",
7810+
"docs": {
7811+
"stability": "experimental"
7812+
},
7813+
"fqn": "jsii-calc.StructPassing",
7814+
"initializer": {},
7815+
"kind": "class",
7816+
"locationInModule": {
7817+
"filename": "lib/compliance.ts",
7818+
"line": 1774
7819+
},
7820+
"methods": [
7821+
{
7822+
"docs": {
7823+
"stability": "experimental"
7824+
},
7825+
"locationInModule": {
7826+
"filename": "lib/compliance.ts",
7827+
"line": 1783
7828+
},
7829+
"name": "howManyVarArgsDidIPass",
7830+
"parameters": [
7831+
{
7832+
"name": "_positional",
7833+
"type": {
7834+
"primitive": "number"
7835+
}
7836+
},
7837+
{
7838+
"name": "inputs",
7839+
"type": {
7840+
"collection": {
7841+
"elementtype": {
7842+
"fqn": "jsii-calc.TopLevelStruct"
7843+
},
7844+
"kind": "array"
7845+
}
7846+
}
7847+
}
7848+
],
7849+
"returns": {
7850+
"type": {
7851+
"primitive": "number"
7852+
}
7853+
},
7854+
"static": true
7855+
},
7856+
{
7857+
"docs": {
7858+
"stability": "experimental"
7859+
},
7860+
"locationInModule": {
7861+
"filename": "lib/compliance.ts",
7862+
"line": 1775
7863+
},
7864+
"name": "roundTrip",
7865+
"parameters": [
7866+
{
7867+
"name": "_positional",
7868+
"type": {
7869+
"primitive": "number"
7870+
}
7871+
},
7872+
{
7873+
"name": "input",
7874+
"type": {
7875+
"fqn": "jsii-calc.TopLevelStruct"
7876+
}
7877+
}
7878+
],
7879+
"returns": {
7880+
"type": {
7881+
"fqn": "jsii-calc.TopLevelStruct"
7882+
}
7883+
},
7884+
"static": true
7885+
}
7886+
],
7887+
"name": "StructPassing"
7888+
},
77597889
"jsii-calc.Sum": {
77607890
"assembly": "jsii-calc",
77617891
"base": "jsii-calc.composition.CompositeOperation",
@@ -8104,6 +8234,80 @@
81048234
],
81058235
"name": "Thrower"
81068236
},
8237+
"jsii-calc.TopLevelStruct": {
8238+
"assembly": "jsii-calc",
8239+
"datatype": true,
8240+
"docs": {
8241+
"stability": "experimental"
8242+
},
8243+
"fqn": "jsii-calc.TopLevelStruct",
8244+
"kind": "interface",
8245+
"locationInModule": {
8246+
"filename": "lib/compliance.ts",
8247+
"line": 1745
8248+
},
8249+
"name": "TopLevelStruct",
8250+
"properties": [
8251+
{
8252+
"abstract": true,
8253+
"docs": {
8254+
"stability": "experimental",
8255+
"summary": "This is a required field."
8256+
},
8257+
"immutable": true,
8258+
"locationInModule": {
8259+
"filename": "lib/compliance.ts",
8260+
"line": 1749
8261+
},
8262+
"name": "required",
8263+
"type": {
8264+
"primitive": "string"
8265+
}
8266+
},
8267+
{
8268+
"abstract": true,
8269+
"docs": {
8270+
"stability": "experimental",
8271+
"summary": "A union to really stress test our serialization."
8272+
},
8273+
"immutable": true,
8274+
"locationInModule": {
8275+
"filename": "lib/compliance.ts",
8276+
"line": 1759
8277+
},
8278+
"name": "secondLevel",
8279+
"type": {
8280+
"union": {
8281+
"types": [
8282+
{
8283+
"primitive": "number"
8284+
},
8285+
{
8286+
"fqn": "jsii-calc.SecondLevelStruct"
8287+
}
8288+
]
8289+
}
8290+
}
8291+
},
8292+
{
8293+
"abstract": true,
8294+
"docs": {
8295+
"stability": "experimental",
8296+
"summary": "You don't have to pass this."
8297+
},
8298+
"immutable": true,
8299+
"locationInModule": {
8300+
"filename": "lib/compliance.ts",
8301+
"line": 1754
8302+
},
8303+
"name": "optional",
8304+
"optional": true,
8305+
"type": {
8306+
"primitive": "string"
8307+
}
8308+
}
8309+
]
8310+
},
81078311
"jsii-calc.UnaryOperation": {
81088312
"abstract": true,
81098313
"assembly": "jsii-calc",
@@ -8851,5 +9055,5 @@
88519055
}
88529056
},
88539057
"version": "0.13.4",
8854-
"fingerprint": "T+Xf0Qtu6HVsykkz6vGH8OhMtRQbt7Jzq4O7Rsv5SeM="
9058+
"fingerprint": "7VlL0zBte+Qeew8qdhvaeS0ZtQtbbT8m3wjT7mDtAoE="
88559059
}

‎packages/jsii-pacmak/bin/jsii-pacmak.ts‎

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
#!/usr/bin/env node
22
import fs = require('fs-extra');
3+
import reflect = require('jsii-reflect');
34
import spec = require('jsii-spec');
45
import os = require('os');
56
import path = require('path');
@@ -136,6 +137,10 @@ import { VERSION_DESC } from '../lib/version';
136137
const tarball = await timers.recordAsync('npm pack', () => {
137138
return npmPack(packageDir, tmpdir);
138139
});
140+
141+
const ts = new reflect.TypeSystem();
142+
const assembly = await ts.loadModule(packageDir);
143+
139144
for (const targetName of targets) {
140145
// if we are targeting a single language, output to outdir, otherwise outdir/<target>
141146
const targetOutputDir = (targets.length > 1 || forceSubdirectory)
@@ -144,7 +149,7 @@ import { VERSION_DESC } from '../lib/version';
144149
logging.debug(`Building ${pkg.name}/${targetName}: ${targetOutputDir}`);
145150

146151
await timers.recordAsync(targetName.toString(), () =>
147-
generateTarget(packageDir, targetName.toString(), targetOutputDir, tarball)
152+
generateTarget(assembly, packageDir, targetName.toString(), targetOutputDir, tarball)
148153
);
149154
}
150155
} finally {
@@ -159,7 +164,7 @@ import { VERSION_DESC } from '../lib/version';
159164
logging.info(`Packaged. ${timers.display()}`);
160165
}
161166

162-
async function generateTarget(packageDir: string, targetName: string, targetOutputDir: string, tarball: string) {
167+
async function generateTarget(assembly: reflect.Assembly, packageDir: string, targetName: string, targetOutputDir: string, tarball: string) {
163168
// ``argv.target`` is guaranteed valid by ``yargs`` through the ``choices`` directive.
164169
const targetConstructor = targetConstructors[targetName];
165170
if (!targetConstructor) {
@@ -169,6 +174,7 @@ import { VERSION_DESC } from '../lib/version';
169174
const target = new targetConstructor({
170175
targetName,
171176
packageDir,
177+
assembly,
172178
fingerprint: argv.fingerprint,
173179
force: argv.force,
174180
arguments: argv

0 commit comments

Comments
 (0)