From 0f5f349d2f6d7d916c59e4f13e0c5195cc0f80c9 Mon Sep 17 00:00:00 2001 From: Romain Marcadier Date: Thu, 18 Nov 2021 14:33:32 +0100 Subject: [PATCH] fix: C# NamespaceDoc emitted to wrong location (#3183) The namespace determination for the `NamespaceDoc` class generated for submodules was incorrect, as the `jsiiNs` passed to the type resovler was absolute, instead of being the required assembly-relative form. Additionally, removed the `readme` and `locationInSource` property from submodule descriptors included in the jsii assembly under `dependencyClosure`, as those are not necessary and needlessly bloat the assembly document. --- 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 --- packages/@jsii/spec/lib/assembly.ts | 18 +++- .../jsii-calc-lib/lib/submodule/README.md | 3 + .../@scope/jsii-calc-lib/test/assembly.jsii | 5 +- packages/jsii-calc/test/assembly.jsii | 6 +- .../lib/targets/dotnet/dotnetgenerator.ts | 6 +- .../__snapshots__/target-dotnet.test.ts.snap | 100 ++++++++++-------- .../__snapshots__/target-java.test.ts.snap | 11 ++ .../__snapshots__/target-python.test.ts.snap | 5 + packages/jsii/lib/assembler.ts | 31 +++++- 9 files changed, 129 insertions(+), 56 deletions(-) create mode 100644 packages/@scope/jsii-calc-lib/lib/submodule/README.md diff --git a/packages/@jsii/spec/lib/assembly.ts b/packages/@jsii/spec/lib/assembly.ts index 07d483a515..a60a88d807 100644 --- a/packages/@jsii/spec/lib/assembly.ts +++ b/packages/@jsii/spec/lib/assembly.ts @@ -3,7 +3,10 @@ export const SPEC_FILE_NAME = '.jsii'; /** * A JSII assembly specification. */ -export interface Assembly extends AssemblyConfiguration, Documentable { +export interface Assembly + extends AssemblyConfiguration, + Documentable, + ReadMeContainer { /** * The version of the spec schema */ @@ -120,7 +123,7 @@ export interface Assembly extends AssemblyConfiguration, Documentable { * * @default none */ - dependencyClosure?: { [assembly: string]: AssemblyConfiguration }; + dependencyClosure?: { [assembly: string]: DependencyConfiguration }; /** * List if bundled dependencies (these are not expected to be jsii @@ -157,6 +160,10 @@ export interface AssemblyConfiguration extends Targetable { submodules?: { [fqn: string]: Submodule }; } +export interface DependencyConfiguration extends Targetable { + submodules?: { [fqn: string]: Targetable }; +} + /** * A targetable module-like thing * @@ -170,7 +177,12 @@ export interface Targetable { * @default none */ targets?: AssemblyTargets; +} +/** + * Elements that can contain a `readme` property. + */ +export interface ReadMeContainer { /** * The readme document for this module (if any). * @@ -192,7 +204,7 @@ export interface ReadMe { * The difference between a top-level module (the assembly) and a submodule is * that the submodule is annotated with its location in the repository. */ -export type Submodule = SourceLocatable & Targetable; +export type Submodule = ReadMeContainer & SourceLocatable & Targetable; /** * Versions of the JSII Assembly Specification. diff --git a/packages/@scope/jsii-calc-lib/lib/submodule/README.md b/packages/@scope/jsii-calc-lib/lib/submodule/README.md new file mode 100644 index 0000000000..b388cd9eb8 --- /dev/null +++ b/packages/@scope/jsii-calc-lib/lib/submodule/README.md @@ -0,0 +1,3 @@ +# Submodule Readme + +This is a submodule readme. diff --git a/packages/@scope/jsii-calc-lib/test/assembly.jsii b/packages/@scope/jsii-calc-lib/test/assembly.jsii index 981dcd29f1..131d74d634 100644 --- a/packages/@scope/jsii-calc-lib/test/assembly.jsii +++ b/packages/@scope/jsii-calc-lib/test/assembly.jsii @@ -95,6 +95,9 @@ "filename": "lib/index.ts", "line": 130 }, + "readme": { + "markdown": "# Submodule Readme\n\nThis is a submodule readme.\n" + }, "targets": { "dotnet": { "namespace": "Amazon.JSII.Tests.CustomSubmoduleName" @@ -948,5 +951,5 @@ } }, "version": "0.0.0", - "fingerprint": "LN1bs46m2O4aR6AhHvi6UDh/f90EoUT3n5apMPzr9+c=" + "fingerprint": "fCLOsQQLslSg+W8rn7XDZ1RSenH5QeaoTna+j7o+URc=" } diff --git a/packages/jsii-calc/test/assembly.jsii b/packages/jsii-calc/test/assembly.jsii index 4a90175111..597752aca0 100644 --- a/packages/jsii-calc/test/assembly.jsii +++ b/packages/jsii-calc/test/assembly.jsii @@ -95,10 +95,6 @@ "@scope/jsii-calc-lib": { "submodules": { "@scope/jsii-calc-lib.submodule": { - "locationInModule": { - "filename": "lib/index.ts", - "line": 130 - }, "targets": { "dotnet": { "namespace": "Amazon.JSII.Tests.CustomSubmoduleName" @@ -16779,5 +16775,5 @@ } }, "version": "3.20.120", - "fingerprint": "XZczlgiEPAQC/n86Dqa40vixNH4txnPoyuF1Q+jR6I0=" + "fingerprint": "WpcHNV2L+v3B7Ou+8xsRMzye4rf1U+SURQA97A3JT6g=" } diff --git a/packages/jsii-pacmak/lib/targets/dotnet/dotnetgenerator.ts b/packages/jsii-pacmak/lib/targets/dotnet/dotnetgenerator.ts index b6dd9bacce..fa15aa74ed 100644 --- a/packages/jsii-pacmak/lib/targets/dotnet/dotnetgenerator.ts +++ b/packages/jsii-pacmak/lib/targets/dotnet/dotnetgenerator.ts @@ -159,7 +159,9 @@ export class DotNetGenerator extends Generator { const dotnetNs = this.typeresolver.resolveNamespace( this.assembly, this.assembly.name, - jsiiNs, + // Strip the `${assmName}.` prefix here, as the "assembly-relative" NS + // is expected by `this.typeResolver.resovleNamespace`. + jsiiNs.substr(this.assembly.name.length + 1), ); this.emitNamespaceDocs(dotnetNs, jsiiNs, submodule); } @@ -1158,7 +1160,7 @@ export class DotNetGenerator extends Generator { private emitNamespaceDocs( namespace: string, jsiiFqn: string, - docSource: spec.Targetable, + docSource: spec.Targetable & spec.ReadMeContainer, ) { if (!docSource.readme) { return; diff --git a/packages/jsii-pacmak/test/generated-code/__snapshots__/target-dotnet.test.ts.snap b/packages/jsii-pacmak/test/generated-code/__snapshots__/target-dotnet.test.ts.snap index 4f5d46a03d..48f7cdd035 100644 --- a/packages/jsii-pacmak/test/generated-code/__snapshots__/target-dotnet.test.ts.snap +++ b/packages/jsii-pacmak/test/generated-code/__snapshots__/target-dotnet.test.ts.snap @@ -995,6 +995,7 @@ exports[`Generated code for "@scope/jsii-calc-lib": / 1`] = ` ┃ ┗━ 📁 CustomSubmoduleName ┃ ┣━ 📄 IReflectable.cs ┃ ┣━ 📄 IReflectableEntry.cs + ┃ ┣━ 📄 NamespaceDoc.cs ┃ ┣━ 📄 NestingClass.cs ┃ ┣━ 📄 ReflectableEntry.cs ┃ ┗━ 📄 Reflector.cs @@ -2249,6 +2250,24 @@ namespace Amazon.JSII.Tests.CustomSubmoduleName `; +exports[`Generated code for "@scope/jsii-calc-lib": /dotnet/Amazon.JSII.Tests.CalculatorPackageId.LibPackageId/Amazon/JSII/Tests/CustomSubmoduleName/NamespaceDoc.cs 1`] = ` +#pragma warning disable CS0672,CS0809,CS1591 + +namespace Amazon.JSII.Tests.CustomSubmoduleName +{ + /// + ///

Submodule Readme

+ /// + /// This is a submodule readme. + /// + [System.ComponentModel.EditorBrowsable(System.ComponentModel.EditorBrowsableState.Never)] + public class NamespaceDoc + { + } +} + +`; + exports[`Generated code for "@scope/jsii-calc-lib": /dotnet/Amazon.JSII.Tests.CalculatorPackageId.LibPackageId/Amazon/JSII/Tests/CustomSubmoduleName/NestingClass.cs 1`] = ` using Amazon.JSII.Runtime.Deputy; @@ -2894,11 +2913,6 @@ exports[`Generated code for "jsii-calc": / 1`] = ` ┃ ┣━ 📄 Jsii487Derived.cs ┃ ┣━ 📄 Jsii496Derived.cs ┃ ┣━ 📄 JsiiAgent.cs - ┃ ┣━ 📁 JsiiCalc - ┃ ┃ ┗━ 📁 Submodule - ┃ ┃ ┣━ 📁 Isolated - ┃ ┃ ┃ ┗━ 📄 NamespaceDoc.cs - ┃ ┃ ┗━ 📄 NamespaceDoc.cs ┃ ┣━ 📄 JSObjectLiteralForInterface.cs ┃ ┣━ 📄 JSObjectLiteralToNative.cs ┃ ┣━ 📄 JSObjectLiteralToNativeClass.cs @@ -3035,8 +3049,10 @@ exports[`Generated code for "jsii-calc": / 1`] = ` ┃ ┃ ┣━ 📄 Default.cs ┃ ┃ ┣━ 📄 IDefault.cs ┃ ┃ ┣━ 📁 Isolated - ┃ ┃ ┃ ┗━ 📄 Kwargs.cs + ┃ ┃ ┃ ┣━ 📄 Kwargs.cs + ┃ ┃ ┃ ┗━ 📄 NamespaceDoc.cs ┃ ┃ ┣━ 📄 MyClass.cs + ┃ ┃ ┣━ 📄 NamespaceDoc.cs ┃ ┃ ┣━ 📁 NestedSubmodule ┃ ┃ ┃ ┣━ 📁 DeeplyNested ┃ ┃ ┃ ┃ ┗━ 📄 INamespaced.cs @@ -12097,42 +12113,6 @@ namespace Amazon.JSII.Tests.CalculatorNamespace `; -exports[`Generated code for "jsii-calc": /dotnet/Amazon.JSII.Tests.CalculatorPackageId/Amazon/JSII/Tests/CalculatorNamespace/JsiiCalc/Submodule/Isolated/NamespaceDoc.cs 1`] = ` -#pragma warning disable CS0672,CS0809,CS1591 - -namespace Amazon.JSII.Tests.CalculatorNamespace.JsiiCalc.Submodule.Isolated -{ - /// - ///

Read you, read me

- /// - /// This is the readme of the jsii-calc.submodule.isolated module. - /// - [System.ComponentModel.EditorBrowsable(System.ComponentModel.EditorBrowsableState.Never)] - public class NamespaceDoc - { - } -} - -`; - -exports[`Generated code for "jsii-calc": /dotnet/Amazon.JSII.Tests.CalculatorPackageId/Amazon/JSII/Tests/CalculatorNamespace/JsiiCalc/Submodule/NamespaceDoc.cs 1`] = ` -#pragma warning disable CS0672,CS0809,CS1591 - -namespace Amazon.JSII.Tests.CalculatorNamespace.JsiiCalc.Submodule -{ - /// - ///

Read you, read me

- /// - /// This is the readme of the jsii-calc.submodule module. - /// - [System.ComponentModel.EditorBrowsable(System.ComponentModel.EditorBrowsableState.Never)] - public class NamespaceDoc - { - } -} - -`; - exports[`Generated code for "jsii-calc": /dotnet/Amazon.JSII.Tests.CalculatorPackageId/Amazon/JSII/Tests/CalculatorNamespace/JsonFormatter.cs 1`] = ` using Amazon.JSII.Runtime.Deputy; @@ -16945,6 +16925,24 @@ namespace Amazon.JSII.Tests.CalculatorNamespace.Submodule.Isolated `; +exports[`Generated code for "jsii-calc": /dotnet/Amazon.JSII.Tests.CalculatorPackageId/Amazon/JSII/Tests/CalculatorNamespace/Submodule/Isolated/NamespaceDoc.cs 1`] = ` +#pragma warning disable CS0672,CS0809,CS1591 + +namespace Amazon.JSII.Tests.CalculatorNamespace.Submodule.Isolated +{ + /// + ///

Read you, read me

+ /// + /// This is the readme of the jsii-calc.submodule.isolated module. + /// + [System.ComponentModel.EditorBrowsable(System.ComponentModel.EditorBrowsableState.Never)] + public class NamespaceDoc + { + } +} + +`; + exports[`Generated code for "jsii-calc": /dotnet/Amazon.JSII.Tests.CalculatorPackageId/Amazon/JSII/Tests/CalculatorNamespace/Submodule/MyClass.cs 1`] = ` using Amazon.JSII.Runtime.Deputy; @@ -17015,6 +17013,24 @@ namespace Amazon.JSII.Tests.CalculatorNamespace.Submodule `; +exports[`Generated code for "jsii-calc": /dotnet/Amazon.JSII.Tests.CalculatorPackageId/Amazon/JSII/Tests/CalculatorNamespace/Submodule/NamespaceDoc.cs 1`] = ` +#pragma warning disable CS0672,CS0809,CS1591 + +namespace Amazon.JSII.Tests.CalculatorNamespace.Submodule +{ + /// + ///

Read you, read me

+ /// + /// This is the readme of the jsii-calc.submodule module. + /// + [System.ComponentModel.EditorBrowsable(System.ComponentModel.EditorBrowsableState.Never)] + public class NamespaceDoc + { + } +} + +`; + exports[`Generated code for "jsii-calc": /dotnet/Amazon.JSII.Tests.CalculatorPackageId/Amazon/JSII/Tests/CalculatorNamespace/Submodule/NestedSubmodule/DeeplyNested/INamespaced.cs 1`] = ` using Amazon.JSII.Runtime.Deputy; diff --git a/packages/jsii-pacmak/test/generated-code/__snapshots__/target-java.test.ts.snap b/packages/jsii-pacmak/test/generated-code/__snapshots__/target-java.test.ts.snap index cef9f9ae79..313cb7c0e0 100644 --- a/packages/jsii-pacmak/test/generated-code/__snapshots__/target-java.test.ts.snap +++ b/packages/jsii-pacmak/test/generated-code/__snapshots__/target-java.test.ts.snap @@ -1418,6 +1418,7 @@ exports[`Generated code for "@scope/jsii-calc-lib": / 1`] = ` ┃ ┣━ 📁 custom_submodule_name ┃ ┃ ┣━ 📄 IReflectable.java ┃ ┃ ┣━ 📄 NestingClass.java + ┃ ┃ ┣━ 📄 package-info.java ┃ ┃ ┣━ 📄 ReflectableEntry.java ┃ ┃ ┗━ 📄 Reflector.java ┃ ┗━ 📁 lib @@ -2249,6 +2250,16 @@ public class Reflector extends software.amazon.jsii.JsiiObject { `; +exports[`Generated code for "@scope/jsii-calc-lib": /java/src/main/java/software/amazon/jsii/tests/calculator/custom_submodule_name/package-info.java 1`] = ` +/** + *

Submodule Readme

+ *

+ * This is a submodule readme. + */ +package software.amazon.jsii.tests.calculator.custom_submodule_name; + +`; + exports[`Generated code for "@scope/jsii-calc-lib": /java/src/main/java/software/amazon/jsii/tests/calculator/lib/$Module.java 1`] = ` package software.amazon.jsii.tests.calculator.lib; diff --git a/packages/jsii-pacmak/test/generated-code/__snapshots__/target-python.test.ts.snap b/packages/jsii-pacmak/test/generated-code/__snapshots__/target-python.test.ts.snap index 6485f5a72f..7f1c6c9856 100644 --- a/packages/jsii-pacmak/test/generated-code/__snapshots__/target-python.test.ts.snap +++ b/packages/jsii-pacmak/test/generated-code/__snapshots__/target-python.test.ts.snap @@ -1863,6 +1863,11 @@ publication.publish() exports[`Generated code for "@scope/jsii-calc-lib": /python/src/scope/jsii_calc_lib/_jsii/jsii-calc-lib@0.0.0.jsii.tgz 1`] = `python/src/scope/jsii_calc_lib/_jsii/jsii-calc-lib@0.0.0.jsii.tgz is a tarball`; exports[`Generated code for "@scope/jsii-calc-lib": /python/src/scope/jsii_calc_lib/custom_submodule_name/__init__.py 1`] = ` +''' +# Submodule Readme + +This is a submodule readme. +''' import abc import builtins import datetime diff --git a/packages/jsii/lib/assembler.ts b/packages/jsii/lib/assembler.ts index 257d3d2012..200a3b1b50 100644 --- a/packages/jsii/lib/assembler.ts +++ b/packages/jsii/lib/assembler.ts @@ -3100,19 +3100,44 @@ function noEmptyDict( } function toDependencyClosure(assemblies: readonly spec.Assembly[]): { - [name: string]: spec.AssemblyConfiguration; + [name: string]: spec.DependencyConfiguration; } { - const result: { [name: string]: spec.AssemblyTargets } = {}; + const result: { [name: string]: spec.DependencyConfiguration } = {}; for (const assembly of assemblies) { if (!assembly.targets) { continue; } result[assembly.name] = { - submodules: assembly.submodules, + submodules: cleanUp(assembly.submodules), targets: assembly.targets, }; } return result; + + /** + * Removes unneeded fields from the entries part of the `dependencyClosure` + * property. Fields such as `readme` are not necessary and can bloat up the + * assembly object. + * + * This removes the `readme` and `locationInModule` fields from the submodule + * descriptios if present. + * + * @param submodules the submodules list to clean up. + * + * @returns the cleaned up submodules list. + */ + function cleanUp( + submodules: spec.Assembly['submodules'], + ): spec.DependencyConfiguration['submodules'] { + if (submodules == null) { + return submodules; + } + const result: spec.DependencyConfiguration['submodules'] = {}; + for (const [fqn, { targets }] of Object.entries(submodules)) { + result[fqn] = { targets }; + } + return result; + } } function toSubmoduleDeclarations(