From 41f395a013a5007fa314ba6e0b7ed8a539ed2c95 Mon Sep 17 00:00:00 2001
From: Congcong Cai <congcongcai0907@163.com>
Date: Fri, 17 Nov 2023 13:43:33 +0800
Subject: [PATCH] fix: add diagnose when getter is not as accessible as the
 setter (#2801)

---
 src/diagnosticMessages.json              |  2 +-
 src/program.ts                           | 18 ++++++++++++++++++
 src/resolver.ts                          | 22 +---------------------
 tests/compiler/getter-setter-errors.json |  8 ++++++++
 tests/compiler/getter-setter-errors.ts   | 16 ++++++++++++++++
 5 files changed, 44 insertions(+), 22 deletions(-)
 create mode 100644 tests/compiler/getter-setter-errors.json
 create mode 100644 tests/compiler/getter-setter-errors.ts

diff --git a/src/diagnosticMessages.json b/src/diagnosticMessages.json
index 3ff9e9f162..3cc11f3825 100644
--- a/src/diagnosticMessages.json
+++ b/src/diagnosticMessages.json
@@ -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,
@@ -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,
diff --git a/src/program.ts b/src/program.ts
index 3174be4875..febeb1227c 100644
--- a/src/program.ts
+++ b/src/program.ts
@@ -3088,6 +3088,13 @@ export abstract class Element {
     return (this.flags & vis) == (other.flags & vis);
   }
 
+  visibilityNoLessThan(other: Element): bool {
+    if (this.isPublic) return true; // public is a most frequent case
+    if (this.is(CommonFlags.Private)) return other.is(CommonFlags.Private);
+    if (this.is(CommonFlags.Protected)) return other.isAny(CommonFlags.Private | CommonFlags.Protected);
+    return assert(false);
+  }
+
   /** Tests if this element is bound to a class. */
   get isBound(): bool {
     let parent = this.parent;
@@ -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. */
diff --git a/src/resolver.ts b/src/resolver.ts
index b1c2504521..d6177806bd 100644
--- a/src/resolver.ts
+++ b/src/resolver.ts
@@ -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) {
@@ -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
@@ -3707,6 +3686,7 @@ export class Resolver extends DiagnosticEmitter {
         }
       }
     }
+    instance.checkVisibility(this);
     return instance;
   }
 
diff --git a/tests/compiler/getter-setter-errors.json b/tests/compiler/getter-setter-errors.json
new file mode 100644
index 0000000000..81e016bde0
--- /dev/null
+++ b/tests/compiler/getter-setter-errors.json
@@ -0,0 +1,8 @@
+{
+  "asc_flags": [
+  ],
+  "stderr": [
+    "TS2808: Get accessor 'm2' must be at least as accessible as the setter.",
+    "EOF"
+  ]
+}
diff --git a/tests/compiler/getter-setter-errors.ts b/tests/compiler/getter-setter-errors.ts
new file mode 100644
index 0000000000..dd2b191127
--- /dev/null
+++ b/tests/compiler/getter-setter-errors.ts
@@ -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");