Skip to content

Commit

Permalink
fix(pacmak): .NET bindings fail to compile with error CS8120 (#3760)
Browse files Browse the repository at this point in the history
If a type union includes several candidates that are related to each other (A extends B or A implements B), `jsii-pacmak` may generate type checking clauses in a pattern matching `switch` statement in an order such that the compiler identifies dead clauses, which is an error in C#.

This adds a provision to NOT emit such a clause so as to not cause the error. It is worth mentioning that the error cannot be "opted out" of via a `#pragma` directive like a warning would be, which is unfortunate.

Fixes #3759



---

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
  • Loading branch information
RomainMuller authored Sep 22, 2022
1 parent a4d39c6 commit 783ec7f
Show file tree
Hide file tree
Showing 14 changed files with 1,596 additions and 715 deletions.
31 changes: 30 additions & 1 deletion .all-contributorsrc
Original file line number Diff line number Diff line change
Expand Up @@ -1272,7 +1272,8 @@
"avatar_url": "https://avatars.githubusercontent.com/u/66279577?v=4",
"profile": "https://github.com/comcalvi",
"contributions": [
"code"
"code",
"review"
]
},
{
Expand Down Expand Up @@ -1428,6 +1429,34 @@
"contributions": [
"bug"
]
},
{
"login": "DanielMSchmidt",
"name": "Daniel Schmidt",
"avatar_url": "https://avatars.githubusercontent.com/u/1337046?v=4",
"profile": "http://danielmschmidt.de/",
"contributions": [
"bug",
"code"
]
},
{
"login": "kaizencc",
"name": "Kaizen Conroy",
"avatar_url": "https://avatars.githubusercontent.com/u/36202692?v=4",
"profile": "https://github.com/kaizencc",
"contributions": [
"code"
]
},
{
"login": "Naumel",
"name": "Naumel",
"avatar_url": "https://avatars.githubusercontent.com/u/104374999?v=4",
"profile": "https://github.com/Naumel",
"contributions": [
"review"
]
}
],
"repoType": "github",
Expand Down
381 changes: 194 additions & 187 deletions README.md

Large diffs are not rendered by default.

1 change: 1 addition & 0 deletions packages/jsii-calc/lib/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,3 +25,4 @@ export * as cdk16625 from './cdk16625';
export * as jsii3656 from './jsii3656';

export * as anonymous from './anonymous';
export * as union from './union';
21 changes: 21 additions & 0 deletions packages/jsii-calc/lib/union.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
import { IFriendly } from '@scope/jsii-calc-lib';

export interface IResolvable {
resolve(): any;
}

export class Resolvable implements IResolvable {
private constructor() {}

public resolve(): any {
return false;
}
}

export class ConsumesUnion {
public static unionType(param: IResolvable | Resolvable | IFriendly) {
void param;
}

private constructor() {}
}
126 changes: 125 additions & 1 deletion packages/jsii-calc/test/assembly.jsii
Original file line number Diff line number Diff line change
Expand Up @@ -421,6 +421,13 @@
"line": 4
},
"symbolId": "lib/submodule/returns-param/index:"
},
"jsii-calc.union": {
"locationInModule": {
"filename": "lib/index.ts",
"line": 28
},
"symbolId": "lib/union:"
}
},
"targets": {
Expand Down Expand Up @@ -18199,8 +18206,125 @@
"name": "ReturnsSpecialParameter",
"namespace": "submodule.returnsparam",
"symbolId": "lib/submodule/returns-param/index:ReturnsSpecialParameter"
},
"jsii-calc.union.ConsumesUnion": {
"assembly": "jsii-calc",
"docs": {
"stability": "stable"
},
"fqn": "jsii-calc.union.ConsumesUnion",
"kind": "class",
"locationInModule": {
"filename": "lib/union.ts",
"line": 15
},
"methods": [
{
"docs": {
"stability": "stable"
},
"locationInModule": {
"filename": "lib/union.ts",
"line": 16
},
"name": "unionType",
"parameters": [
{
"name": "param",
"type": {
"union": {
"types": [
{
"fqn": "jsii-calc.union.IResolvable"
},
{
"fqn": "jsii-calc.union.Resolvable"
},
{
"fqn": "@scope/jsii-calc-lib.IFriendly"
}
]
}
}
}
],
"static": true
}
],
"name": "ConsumesUnion",
"namespace": "union",
"symbolId": "lib/union:ConsumesUnion"
},
"jsii-calc.union.IResolvable": {
"assembly": "jsii-calc",
"docs": {
"stability": "stable"
},
"fqn": "jsii-calc.union.IResolvable",
"kind": "interface",
"locationInModule": {
"filename": "lib/union.ts",
"line": 3
},
"methods": [
{
"abstract": true,
"docs": {
"stability": "stable"
},
"locationInModule": {
"filename": "lib/union.ts",
"line": 4
},
"name": "resolve",
"returns": {
"type": {
"primitive": "any"
}
}
}
],
"name": "IResolvable",
"namespace": "union",
"symbolId": "lib/union:IResolvable"
},
"jsii-calc.union.Resolvable": {
"assembly": "jsii-calc",
"docs": {
"stability": "stable"
},
"fqn": "jsii-calc.union.Resolvable",
"interfaces": [
"jsii-calc.union.IResolvable"
],
"kind": "class",
"locationInModule": {
"filename": "lib/union.ts",
"line": 7
},
"methods": [
{
"docs": {
"stability": "stable"
},
"locationInModule": {
"filename": "lib/union.ts",
"line": 10
},
"name": "resolve",
"overrides": "jsii-calc.union.IResolvable",
"returns": {
"type": {
"primitive": "any"
}
}
}
],
"name": "Resolvable",
"namespace": "union",
"symbolId": "lib/union:Resolvable"
}
},
"version": "3.20.120",
"fingerprint": "Ze43eowG9ImRufT3MQ8yO+bW8JzOQlZIYtFsjpc960E="
"fingerprint": "OaHwYmdPa8tbAJnlREahLGpRaNNBor2aoG04vsA/SvM="
}
25 changes: 25 additions & 0 deletions packages/jsii-pacmak/lib/targets/dotnet/runtime-type-checking.ts
Original file line number Diff line number Diff line change
Expand Up @@ -172,6 +172,31 @@ abstract class Validation {
code.openBlock(`switch (${expression})`);
for (const type of types) {
validTypes.push(resolver.toDotNetTypeName(type.spec!));

/**
* Filter to remove classes and interfaces from a set of type references that
* are implied by another entry in the set. Practically this is meant to remove
* types from a set if a parent type of it is also present in the set, keeping
* only the most generic declaration.
*
* This is useful because the TypeScript compiler and jsii do not guarantee that
* all entries in a type union are unrelated, but the C# compiler treats dead
* code as an error, and will refuse to compile (error CS8120) a pattern-matching
* switch case if it cannot be matched (for example, if it matches on a child of
* a type that was previously matched on already).
*/
if (
(type.type?.isClassType() || type.type?.isInterfaceType()) &&
types.some(
(other) =>
other !== type &&
other.type != null &&
type.type!.extends(other.type),
)
) {
continue;
}

const typeNames = [resolver.toDotNetType(type.spec!)];
if (typeNames[0] === 'double') {
// For doubles, we accept any numeric value, really...
Expand Down
Loading

0 comments on commit 783ec7f

Please sign in to comment.