From 89b49f03fb6efa19cbe99a9371fbf4ce8abcb431 Mon Sep 17 00:00:00 2001 From: CountBleck Date: Mon, 20 Nov 2023 18:43:14 -0800 Subject: [PATCH 1/2] Report errors from initialize This is unfortunately necessary for the next commit. This is overall bad for DX, since fewer errors are shown to the user at once. --- cli/index.js | 8 +++++++- tests/compiler/covariance.json | 7 +++++++ tests/compiler/covariance.ts | 9 +++++++++ tests/compiler/duplicate-field-errors.json | 10 ++-------- tests/compiler/duplicate-field-errors.ts | 8 -------- tests/compiler/duplicate-identifier-function.json | 7 +++++++ tests/compiler/duplicate-identifier-function.ts | 12 ++++++++++++ tests/compiler/duplicate-identifier.json | 5 +---- tests/compiler/duplicate-identifier.ts | 13 ------------- tests/compiler/unmanaged-errors.json | 3 +-- tests/compiler/unmanaged-errors.ts | 2 -- tests/compiler/unsafe-not-valid.json | 5 +++++ tests/compiler/unsafe-not-valid.ts | 3 +++ tests/compiler/unsafe.json | 1 - tests/compiler/unsafe.ts | 4 ---- 15 files changed, 54 insertions(+), 43 deletions(-) create mode 100644 tests/compiler/covariance.json create mode 100644 tests/compiler/covariance.ts create mode 100644 tests/compiler/duplicate-identifier-function.json create mode 100644 tests/compiler/duplicate-identifier-function.ts create mode 100644 tests/compiler/unsafe-not-valid.json create mode 100644 tests/compiler/unsafe-not-valid.ts diff --git a/cli/index.js b/cli/index.js index 91634a8c01..dca2797988 100644 --- a/cli/index.js +++ b/cli/index.js @@ -716,6 +716,12 @@ export async function main(argv, options) { } stats.initializeTime += stats.end(begin); } + let numErrors = checkDiagnostics(program, stderr, opts.disableWarning, options.reportDiagnostic, stderrColors.enabled); + if (numErrors) { + const err = Error(`${numErrors} initialize error(s)`); + err.stack = err.message; // omit stack + return prepareResult(err); + } // Call afterInitialize transform hook { @@ -740,7 +746,7 @@ export async function main(argv, options) { ? assemblyscript.getBinaryenModuleRef(module) : module.ref ); - let numErrors = checkDiagnostics(program, stderr, opts.disableWarning, options.reportDiagnostic, stderrColors.enabled); + numErrors = checkDiagnostics(program, stderr, opts.disableWarning, options.reportDiagnostic, stderrColors.enabled); if (numErrors) { const err = Error(`${numErrors} compile error(s)`); err.stack = err.message; // omit stack diff --git a/tests/compiler/covariance.json b/tests/compiler/covariance.json new file mode 100644 index 0000000000..5b2153a640 --- /dev/null +++ b/tests/compiler/covariance.json @@ -0,0 +1,7 @@ +{ + "stderr": [ + "TS2394: This overload signature is not compatible with its implementation signature.", + "sibling: Cat | null;", + "sibling: Animal | null;" + ] +} \ No newline at end of file diff --git a/tests/compiler/covariance.ts b/tests/compiler/covariance.ts new file mode 100644 index 0000000000..4cb3d4c035 --- /dev/null +++ b/tests/compiler/covariance.ts @@ -0,0 +1,9 @@ +class Animal { + sibling: Animal | null; +} + +class Cat extends Animal { + sibling: Cat | null; // covariance is unsound +} + +new Cat(); \ No newline at end of file diff --git a/tests/compiler/duplicate-field-errors.json b/tests/compiler/duplicate-field-errors.json index dd6ed9a032..70a641dde0 100644 --- a/tests/compiler/duplicate-field-errors.json +++ b/tests/compiler/duplicate-field-errors.json @@ -3,13 +3,7 @@ ], "stderr": [ "TS2385: Overload signatures must all be public, private or protected.", "public privPub: i32;", "private privPub: i32;", - "TS2442: Types have separate declarations of a private property 'privPriv'.", - "TS2325: Property 'privProt' is private in type 'duplicate-field-errors/A' but not in type 'duplicate-field-errors/B'.", - "TS2325: Property 'privPub' is private in type 'duplicate-field-errors/A' but not in type 'duplicate-field-errors/B'.", - "TS2325: Property 'protPriv' is private in type 'duplicate-field-errors/B' but not in type 'duplicate-field-errors/A'.", - "TS2325: Property 'pubPriv' is private in type 'duplicate-field-errors/B' but not in type 'duplicate-field-errors/A'.", - "TS2444: Property 'pubProt' is protected in type 'duplicate-field-errors/B' but public in type 'duplicate-field-errors/A'.", - "TS2394: This overload signature is not compatible with its implementation signature.", "public method: i32;", "method(): void", - "TS2394: This overload signature is not compatible with its implementation signature.", "sibling: Cat | null;", "sibling: Animal | null;" + "TS2385: Overload signatures must all be public, private or protected.", "protected pubProt: i32;", "public pubProt: i32;", + "TS2416: Property 'method' in type 'duplicate-field-errors/B' is not assignable to the same property in base type 'duplicate-field-errors/A'." ] } diff --git a/tests/compiler/duplicate-field-errors.ts b/tests/compiler/duplicate-field-errors.ts index dd5bc58cac..61979a4166 100644 --- a/tests/compiler/duplicate-field-errors.ts +++ b/tests/compiler/duplicate-field-errors.ts @@ -36,15 +36,7 @@ class B extends A { public method: i32; } -class Animal { - sibling: Animal | null; -} -class Cat extends Animal { - sibling: Cat | null; // covariance is unsound -} - export function test(): void { new A(); new B(); - new Cat(); } diff --git a/tests/compiler/duplicate-identifier-function.json b/tests/compiler/duplicate-identifier-function.json new file mode 100644 index 0000000000..e87c11ab87 --- /dev/null +++ b/tests/compiler/duplicate-identifier-function.json @@ -0,0 +1,7 @@ +{ + "stderr": [ + "TS2300: Duplicate identifier 'e'", "var e: f32", "var e: i32", + "TS2300: Duplicate identifier 'f'", "let f: f32", + "EOF" + ] +} \ No newline at end of file diff --git a/tests/compiler/duplicate-identifier-function.ts b/tests/compiler/duplicate-identifier-function.ts new file mode 100644 index 0000000000..22f201a56a --- /dev/null +++ b/tests/compiler/duplicate-identifier-function.ts @@ -0,0 +1,12 @@ +function baz(): void { + var e: i32; + var e: f32; + { + let f: i32; + let f: f32; + } +} + +baz(); + +ERROR("EOF"); // mark end and ensure fail diff --git a/tests/compiler/duplicate-identifier.json b/tests/compiler/duplicate-identifier.json index a3f82cc818..a19a424726 100644 --- a/tests/compiler/duplicate-identifier.json +++ b/tests/compiler/duplicate-identifier.json @@ -5,9 +5,6 @@ "TS2300: Duplicate identifier 'a'", "var a: f32", "var a: i32", "TS2300: Duplicate identifier 'b'", "b: f32", "b: i32", "TS2300: Duplicate identifier 'c'", "static c: f32", " static c: i32", - "TS2300: Duplicate identifier 'd'", "const d: f32", "const d: i32", - "TS2300: Duplicate identifier 'e'", "var e: f32", "var e: i32", - "TS2300: Duplicate identifier 'f'", "let f: f32", - "EOF" + "TS2300: Duplicate identifier 'd'", "const d: f32", "const d: i32" ] } diff --git a/tests/compiler/duplicate-identifier.ts b/tests/compiler/duplicate-identifier.ts index b9482e8369..aac76606be 100644 --- a/tests/compiler/duplicate-identifier.ts +++ b/tests/compiler/duplicate-identifier.ts @@ -12,16 +12,3 @@ namespace Bar { const d: i32 = 0; const d: f32 = 1; } - -function baz(): void { - var e: i32; - var e: f32; - { - let f: i32; - let f: f32; - } -} - -baz(); - -ERROR("EOF"); // mark end and ensure fail diff --git a/tests/compiler/unmanaged-errors.json b/tests/compiler/unmanaged-errors.json index a636c438c9..022ef729a6 100644 --- a/tests/compiler/unmanaged-errors.json +++ b/tests/compiler/unmanaged-errors.json @@ -3,7 +3,6 @@ ], "stderr": [ "AS207: Unmanaged classes cannot extend managed classes and vice-versa.", "UnmanagedFoo extends ManagedBase", - "AS207: Unmanaged classes cannot extend managed classes and vice-versa.", "ManagedFoo extends UnmanagedBase", - "EOF" + "AS207: Unmanaged classes cannot extend managed classes and vice-versa.", "ManagedFoo extends UnmanagedBase" ] } diff --git a/tests/compiler/unmanaged-errors.ts b/tests/compiler/unmanaged-errors.ts index 2e73894bb7..89f521c565 100644 --- a/tests/compiler/unmanaged-errors.ts +++ b/tests/compiler/unmanaged-errors.ts @@ -8,5 +8,3 @@ class ManagedBaz {} class ManagedFoo extends UnmanagedBase { // AS207 constructor(public baz: ManagedBaz) { super(); } } - -ERROR("EOF"); diff --git a/tests/compiler/unsafe-not-valid.json b/tests/compiler/unsafe-not-valid.json new file mode 100644 index 0000000000..b924c19455 --- /dev/null +++ b/tests/compiler/unsafe-not-valid.json @@ -0,0 +1,5 @@ +{ + "stderr": [ + "AS212: Decorator '@unsafe' is not valid here." + ] +} \ No newline at end of file diff --git a/tests/compiler/unsafe-not-valid.ts b/tests/compiler/unsafe-not-valid.ts new file mode 100644 index 0000000000..26a82a7c82 --- /dev/null +++ b/tests/compiler/unsafe-not-valid.ts @@ -0,0 +1,3 @@ +// Global + +@unsafe var g = 0; // not valid here \ No newline at end of file diff --git a/tests/compiler/unsafe.json b/tests/compiler/unsafe.json index f44bd65446..319dd27cf2 100644 --- a/tests/compiler/unsafe.json +++ b/tests/compiler/unsafe.json @@ -3,7 +3,6 @@ "--noUnsafe" ], "stderr": [ - "AS212: Decorator '@unsafe' is not valid here.", "AS101: Operation is unsafe.", "f1();", "AS101: Operation is unsafe.", "f2();", "AS101: Operation is unsafe.", "= new Foo();", diff --git a/tests/compiler/unsafe.ts b/tests/compiler/unsafe.ts index 29d1bc7353..16efa287ea 100644 --- a/tests/compiler/unsafe.ts +++ b/tests/compiler/unsafe.ts @@ -1,7 +1,3 @@ -// Global - -@unsafe var g = 0; // not valid here - // Function @unsafe function f1(): void {} From 4b4d68c44e23e5c18d400dbb72abba5aad00a363 Mon Sep 17 00:00:00 2001 From: CountBleck Date: Mon, 20 Nov 2023 18:50:29 -0800 Subject: [PATCH 2/2] Forbid duplicate static class and/or namespace members This isn't fully TS compatible, but refactors targeting internal names, scoping, merging, etc. are needed to become more compatible. For instance, if namespace members had unique separators in internal names, then a non-exported namespace member would override a static class member, assuming the names are the same. Note that this change doesn't prevent the compiler from attempting to compile the duplicate global, and hence the previous commit is needed for this to work fully. Barely fixes #2793. --- src/program.ts | 20 ++++++++++++++++++-- tests/compiler/issues/2793.json | 13 +++++++++++++ tests/compiler/issues/2793.ts | 12 ++++++++++++ 3 files changed, 43 insertions(+), 2 deletions(-) create mode 100644 tests/compiler/issues/2793.json create mode 100644 tests/compiler/issues/2793.ts diff --git a/src/program.ts b/src/program.ts index febeb1227c..1db22d6855 100644 --- a/src/program.ts +++ b/src/program.ts @@ -5058,6 +5058,9 @@ function tryMerge(older: Element, newer: Element): DeclaredElement | null { /** Copies the members of `src` to `dest`. */ function copyMembers(src: Element, dest: Element): void { + let program = src.program; + assert(program == dest.program); + let srcMembers = src.members; if (srcMembers) { let destMembers = dest.members; @@ -5065,8 +5068,21 @@ function copyMembers(src: Element, dest: Element): void { // TODO: for (let [memberName, member] of srcMembers) { for (let _keys = Map_keys(srcMembers), i = 0, k = _keys.length; i < k; ++i) { let memberName = unchecked(_keys[i]); - let member = assert(srcMembers.get(memberName)); - destMembers.set(memberName, member); + let srcMember = assert(srcMembers.get(memberName)); + // This isn't TS compatible in every case, but the logic involved in + // merging, scoping, and making internal names is not currently able to + // handle duplicates well. + if (destMembers.has(memberName)) { + let destMember = assert(destMembers.get(memberName)); + program.errorRelated( + DiagnosticCode.Duplicate_identifier_0, + srcMember.declaration.name.range, destMember.declaration.name.range, + memberName + ); + continue; + } + + destMembers.set(memberName, srcMember); } } } diff --git a/tests/compiler/issues/2793.json b/tests/compiler/issues/2793.json new file mode 100644 index 0000000000..8f275f6155 --- /dev/null +++ b/tests/compiler/issues/2793.json @@ -0,0 +1,13 @@ +{ + "stderr": [ + "Duplicate identifier 'bar'.", + "in issues/2793.ts(8,14)", + "in issues/2793.ts(2,10)", + "Duplicate identifier 'baz'.", + "in issues/2793.ts(9,7)", + "in issues/2793.ts(3,10)", + "Duplicate identifier 'qux'.", + "in issues/2793.ts(10,14)", + "in issues/2793.ts(4,18)" + ] +} \ No newline at end of file diff --git a/tests/compiler/issues/2793.ts b/tests/compiler/issues/2793.ts new file mode 100644 index 0000000000..b9f966bc43 --- /dev/null +++ b/tests/compiler/issues/2793.ts @@ -0,0 +1,12 @@ +class Foo { + static bar: i32 = 2; // errors in TS + static baz: i32 = 2; // does not error in TS + private static qux: i32 = 2; // errors in TS +} + +namespace Foo { + export let bar: i32 = 1; + let baz: string = "baz"; + export let qux: i32 = 1; + let hi: i32 = 1; +}