Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Forbid duplicate namespace/static class members #2805

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 7 additions & 1 deletion cli/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
{
Expand All @@ -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
Expand Down
20 changes: 18 additions & 2 deletions src/program.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5058,15 +5058,31 @@ 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;
if (!destMembers) dest.members = destMembers = new Map();
// 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);
}
}
}
Expand Down
7 changes: 7 additions & 0 deletions tests/compiler/covariance.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
{
"stderr": [
"TS2394: This overload signature is not compatible with its implementation signature.",
"sibling: Cat | null;",
"sibling: Animal | null;"
]
}
9 changes: 9 additions & 0 deletions tests/compiler/covariance.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
class Animal {
sibling: Animal | null;
}

class Cat extends Animal {
sibling: Cat | null; // covariance is unsound
}

new Cat();
10 changes: 2 additions & 8 deletions tests/compiler/duplicate-field-errors.json
Original file line number Diff line number Diff line change
Expand Up @@ -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'."
]
}
8 changes: 0 additions & 8 deletions tests/compiler/duplicate-field-errors.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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();
}
7 changes: 7 additions & 0 deletions tests/compiler/duplicate-identifier-function.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
{
"stderr": [
"TS2300: Duplicate identifier 'e'", "var e: f32", "var e: i32",
"TS2300: Duplicate identifier 'f'", "let f: f32",
"EOF"
]
}
12 changes: 12 additions & 0 deletions tests/compiler/duplicate-identifier-function.ts
Original file line number Diff line number Diff line change
@@ -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
5 changes: 1 addition & 4 deletions tests/compiler/duplicate-identifier.json
Original file line number Diff line number Diff line change
Expand Up @@ -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"
]
}
13 changes: 0 additions & 13 deletions tests/compiler/duplicate-identifier.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
13 changes: 13 additions & 0 deletions tests/compiler/issues/2793.json
Original file line number Diff line number Diff line change
@@ -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)"
]
}
12 changes: 12 additions & 0 deletions tests/compiler/issues/2793.ts
Original file line number Diff line number Diff line change
@@ -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;
}
3 changes: 1 addition & 2 deletions tests/compiler/unmanaged-errors.json
Original file line number Diff line number Diff line change
Expand Up @@ -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"
]
}
2 changes: 0 additions & 2 deletions tests/compiler/unmanaged-errors.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,5 +8,3 @@ class ManagedBaz {}
class ManagedFoo extends UnmanagedBase { // AS207
constructor(public baz: ManagedBaz) { super(); }
}

ERROR("EOF");
5 changes: 5 additions & 0 deletions tests/compiler/unsafe-not-valid.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
{
"stderr": [
"AS212: Decorator '@unsafe' is not valid here."
]
}
3 changes: 3 additions & 0 deletions tests/compiler/unsafe-not-valid.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
// Global

@unsafe var g = 0; // not valid here
1 change: 0 additions & 1 deletion tests/compiler/unsafe.json
Original file line number Diff line number Diff line change
Expand Up @@ -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();",
Expand Down
4 changes: 0 additions & 4 deletions tests/compiler/unsafe.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,3 @@
// Global

@unsafe var g = 0; // not valid here

// Function

@unsafe function f1(): void {}
Expand Down