Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Next Next commit
fix: add diagnose for getter and setter with difference visibility
  • Loading branch information
HerrCai0907 committed Nov 16, 2023
commit f3a48eae3ffb955a7af05850982494917a96f0a2
2 changes: 1 addition & 1 deletion src/diagnosticMessages.json
Original file line number Diff line number Diff line change
Expand Up @@ -151,7 +151,6 @@
"Operator '{0}' cannot be applied to types '{1}' and '{2}'.": 2365,
"A 'super' call must be the first statement in the constructor.": 2376,
"Constructors for derived classes must contain a 'super' call.": 2377,
"Getter and setter accessors do not agree in visibility.": 2379,
"'get' and 'set' accessor must have the same type.": 2380,
"Overload signatures must all be public, private or protected.": 2385,
"Constructor implementation is missing.": 2390,
Expand Down Expand Up @@ -200,6 +199,7 @@
"Duplicate property '{0}'.": 2718,
"Property '{0}' is missing in type '{1}' but required in type '{2}'.": 2741,
"Type '{0}' has no call signatures.": 2757,
"Get accessor '{0}' must be at least as accessible as the setter.": 2808,
"This member cannot have an 'override' modifier because it is not declared in the base class '{0}'.": 4117,

"File '{0}' not found.": 6054,
Expand Down
18 changes: 18 additions & 0 deletions src/program.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3088,6 +3088,13 @@ export abstract class Element {
return (this.flags & vis) == (other.flags & vis);
}

visibilityNoLessThan(other: Element): bool {
if (this.is(CommonFlags.Private)) return other.is(CommonFlags.Private);
if (this.is(CommonFlags.Protected)) return other.isAny(CommonFlags.Private | CommonFlags.Protected);
assert(this.isPublic);
return true;
}

/** Tests if this element is bound to a class. */
get isBound(): bool {
let parent = this.parent;
Expand Down Expand Up @@ -4174,6 +4181,17 @@ export class Property extends VariableLikeElement {
get isField(): bool {
return this.prototype.isField;
}

checkVisibility(diag: DiagnosticEmitter): void {
let propertyGetter = this.getterInstance;
let propertySetter = this.setterInstance;
if (propertyGetter && propertySetter && !propertyGetter.visibilityNoLessThan(propertySetter)) {
diag.errorRelated(
DiagnosticCode.Get_accessor_0_must_be_at_least_as_accessible_as_the_setter,
propertyGetter.identifierNode.range, propertySetter.identifierNode.range, propertyGetter.identifierNode.text
);
}
}
}

/** A resolved index signature. */
Expand Down
22 changes: 1 addition & 21 deletions src/resolver.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3384,7 +3384,6 @@ export class Resolver extends DiagnosticEmitter {
// Resolve instance members
let prototype = instance.prototype;
let instanceMemberPrototypes = prototype.instanceMembers;
let properties = new Array<Property>();
if (instanceMemberPrototypes) {
// TODO: for (let member of instanceMemberPrototypes.values()) {
for (let _values = Map_values(instanceMemberPrototypes), i = 0, k = _values.length; i < k; ++i) {
Expand Down Expand Up @@ -3464,26 +3463,6 @@ export class Resolver extends DiagnosticEmitter {
}
}

// Check that property getters and setters match
for (let i = 0, k = properties.length; i < k; ++i) {
let property = properties[i];
let propertyGetter = property.getterInstance;
if (!propertyGetter) {
this.error(
DiagnosticCode.Property_0_only_has_a_setter_and_is_missing_a_getter,
property.identifierNode.range, property.name
);
} else {
let propertySetter = property.setterInstance;
if (propertySetter && !propertyGetter.visibilityEquals(propertySetter)) {
this.errorRelated(
DiagnosticCode.Getter_and_setter_accessors_do_not_agree_in_visibility,
propertyGetter.identifierNode.range, propertySetter.identifierNode.range
);
}
}
}

if (instance.kind != ElementKind.Interface) {

// Check that all required members are implemented
Expand Down Expand Up @@ -3707,6 +3686,7 @@ export class Resolver extends DiagnosticEmitter {
}
}
}
instance.checkVisibility(this);
return instance;
}

Expand Down
8 changes: 8 additions & 0 deletions tests/compiler/getter-setter-errors.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
{
"asc_flags": [
],
"stderr": [
"TS2808: Get accessor 'm2' must be at least as accessible as the setter.",
"EOF"
]
}
16 changes: 16 additions & 0 deletions tests/compiler/getter-setter-errors.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
class GetSetWithoutDifferenceVisibility {
public get m1(): i32 {
return 1;
}
private set m1(v: i32) {}

private get m2(): i32 {
return 1;
}
public set m2(v: i32) {}
}

new GetSetWithoutDifferenceVisibility().m1; // m1 is valid
new GetSetWithoutDifferenceVisibility().m2; // m2 is invalid

ERROR("EOF");