chore: add lint rule to prevent unnecessary public modifiers (#1557)

.eslintrc.json

@@ -70,6 +70,12 @@
     ],
     "no-prototype-builtins": "off",
     "@typescript-eslint/ban-types": "off",
+    "@typescript-eslint/explicit-member-accessibility": [
+      "error",
+      {
+        "accessibility": "no-public"
+      }
+    ],
     "@typescript-eslint/explicit-module-boundary-types": "off",
     "@typescript-eslint/no-empty-function": "off",
     "@typescript-eslint/no-explicit-any": "error",

src/metrics_server/connection_metrics.spec.ts

@@ -70,7 +70,7 @@ const LEGACY_REPORT: HourlyConnectionMetricsReport = {
 };
 
 class FakeConnectionsTable implements InsertableTable<ConnectionRow> {
-  public rows: ConnectionRow[] | undefined;
+  rows: ConnectionRow[] | undefined;
 
   async insert(rows: ConnectionRow[]) {
     this.rows = rows;

src/metrics_server/feature_metrics.spec.ts

@@ -17,7 +17,7 @@ import {InsertableTable} from './infrastructure/table';
 import {DailyFeatureMetricsReport} from './model';
 
 class FakeFeaturesTable implements InsertableTable<FeatureRow> {
-  public rows: FeatureRow[] | undefined;
+  rows: FeatureRow[] | undefined;
 
   async insert(rows: FeatureRow[]) {
     this.rows = rows;

src/shadowbox/infrastructure/clock.ts

@@ -31,7 +31,7 @@ export class RealClock implements Clock {
 // Fake clock where you manually set what is "now" and can trigger the scheduled callbacks.
 // Useful for tests.
 export class ManualClock implements Clock {
-  public nowMs = 0;
+  nowMs = 0;
   private callbacks = [] as (() => void)[];
 
   now(): number {

src/shadowbox/infrastructure/json_config.ts

@@ -89,7 +89,7 @@ export class DelayedConfig<T> implements JsonConfig<T> {
 // InMemoryConfig is a JsonConfig backed by an internal member variable. Useful for testing.
 export class InMemoryConfig<T> implements JsonConfig<T> {
   // Holds the data JSON as it was when `write()` was called.
-  public mostRecentWrite: T;
+  mostRecentWrite: T;
   constructor(private dataJson: T) {
     this.mostRecentWrite = this.dataJson;
   }

src/shadowbox/server/manager_service.ts

@@ -253,7 +253,7 @@ export class ShadowsocksManagerService {
     private metricsPublisher: SharedMetricsPublisher
   ) {}
 
-  public renameServer(req: RequestType, res: ResponseType, next: restify.Next): void {
+  renameServer(req: RequestType, res: ResponseType, next: restify.Next): void {
     logging.debug(`renameServer request ${JSON.stringify(req.params)}`);
     const name = req.params.name;
     if (!name) {
@@ -275,7 +275,7 @@ export class ShadowsocksManagerService {
     next();
   }
 
-  public getServer(req: RequestType, res: ResponseType, next: restify.Next): void {
+  getServer(req: RequestType, res: ResponseType, next: restify.Next): void {
     res.send(HttpSuccess.OK, {
       name: this.serverConfig.data().name || this.defaultServerName,
       serverId: this.serverConfig.data().serverId,
@@ -291,7 +291,7 @@ export class ShadowsocksManagerService {
   }
 
   // Changes the server's hostname.  Hostname must be a valid domain or IP address
-  public setHostnameForAccessKeys(req: RequestType, res: ResponseType, next: restify.Next): void {
+  setHostnameForAccessKeys(req: RequestType, res: ResponseType, next: restify.Next): void {
     logging.debug(`changeHostname request: ${JSON.stringify(req.params)}`);
 
     const hostname = req.params.hostname;
@@ -329,7 +329,7 @@ export class ShadowsocksManagerService {
   }
 
   // Get a access key
-  public getAccessKey(req: RequestType, res: ResponseType, next: restify.Next): void {
+  getAccessKey(req: RequestType, res: ResponseType, next: restify.Next): void {
     try {
       logging.debug(`getAccessKey request ${JSON.stringify(req.params)}`);
       const accessKeyId = validateAccessKeyId(req.params.id);
@@ -349,7 +349,7 @@ export class ShadowsocksManagerService {
   }
 
   // Lists all access keys
-  public listAccessKeys(req: RequestType, res: ResponseType, next: restify.Next): void {
+  listAccessKeys(req: RequestType, res: ResponseType, next: restify.Next): void {
     logging.debug(`listAccessKeys request ${JSON.stringify(req.params)}`);
     const response = {accessKeys: []};
     for (const accessKey of this.accessKeys.listAccessKeys()) {
@@ -394,11 +394,7 @@ export class ShadowsocksManagerService {
   }
 
   // Creates a new access key
-  public async createNewAccessKey(
+  async createNewAccessKey(req: RequestType, res: ResponseType, next: restify.Next): Promise<void> {
-    req: RequestType,
-    res: ResponseType,
-    next: restify.Next
-  ): Promise<void> {
     try {
       logging.debug(`createNewAccessKey request ${JSON.stringify(req.params)}`);
       if (req.params.id) {
@@ -420,11 +416,7 @@ export class ShadowsocksManagerService {
   }
 
   // Creates an access key with a specific identifier
-  public async createAccessKey(
+  async createAccessKey(req: RequestType, res: ResponseType, next: restify.Next): Promise<void> {
-    req: RequestType,
-    res: ResponseType,
-    next: restify.Next
-  ): Promise<void> {
     try {
       logging.debug(`createAccessKey request ${JSON.stringify(req.params)}`);
       const accessKeyId = validateAccessKeyId(req.params.id);
@@ -445,7 +437,7 @@ export class ShadowsocksManagerService {
   }
 
   // Sets the default ports for new access keys
-  public async setPortForNewAccessKeys(
+  async setPortForNewAccessKeys(
     req: RequestType,
     res: ResponseType,
     next: restify.Next
@@ -477,7 +469,7 @@ export class ShadowsocksManagerService {
   }
 
   // Removes an existing access key
-  public removeAccessKey(req: RequestType, res: ResponseType, next: restify.Next): void {
+  removeAccessKey(req: RequestType, res: ResponseType, next: restify.Next): void {
     try {
       logging.debug(`removeAccessKey request ${JSON.stringify(req.params)}`);
       const accessKeyId = validateAccessKeyId(req.params.id);
@@ -495,7 +487,7 @@ export class ShadowsocksManagerService {
     }
   }
 
-  public renameAccessKey(req: RequestType, res: ResponseType, next: restify.Next): void {
+  renameAccessKey(req: RequestType, res: ResponseType, next: restify.Next): void {
     try {
       logging.debug(`renameAccessKey request ${JSON.stringify(req.params)}`);
       const accessKeyId = validateAccessKeyId(req.params.id);
@@ -526,7 +518,7 @@ export class ShadowsocksManagerService {
     }
   }
 
-  public async setAccessKeyDataLimit(req: RequestType, res: ResponseType, next: restify.Next) {
+  async setAccessKeyDataLimit(req: RequestType, res: ResponseType, next: restify.Next) {
     try {
       logging.debug(`setAccessKeyDataLimit request ${JSON.stringify(req.params)}`);
       const accessKeyId = validateAccessKeyId(req.params.id);
@@ -545,7 +537,7 @@ export class ShadowsocksManagerService {
     }
   }
 
-  public async removeAccessKeyDataLimit(req: RequestType, res: ResponseType, next: restify.Next) {
+  async removeAccessKeyDataLimit(req: RequestType, res: ResponseType, next: restify.Next) {
     try {
       logging.debug(`removeAccessKeyDataLimit request ${JSON.stringify(req.params)}`);
       const accessKeyId = validateAccessKeyId(req.params.id);
@@ -563,7 +555,7 @@ export class ShadowsocksManagerService {
     }
   }
 
-  public async setDefaultDataLimit(req: RequestType, res: ResponseType, next: restify.Next) {
+  async setDefaultDataLimit(req: RequestType, res: ResponseType, next: restify.Next) {
     try {
       logging.debug(`setDefaultDataLimit request ${JSON.stringify(req.params)}`);
       const limit = validateDataLimit(req.params.limit);
@@ -583,7 +575,7 @@ export class ShadowsocksManagerService {
     }
   }
 
-  public async removeDefaultDataLimit(req: RequestType, res: ResponseType, next: restify.Next) {
+  async removeDefaultDataLimit(req: RequestType, res: ResponseType, next: restify.Next) {
     try {
       logging.debug(`removeDefaultDataLimit request ${JSON.stringify(req.params)}`);
       // Enforcement is done asynchronously in the proxy server.  This is transparent to the manager
@@ -599,7 +591,7 @@ export class ShadowsocksManagerService {
     }
   }
 
-  public async getDataUsage(req: RequestType, res: ResponseType, next: restify.Next) {
+  async getDataUsage(req: RequestType, res: ResponseType, next: restify.Next) {
     try {
       logging.debug(`getDataUsage request ${JSON.stringify(req.params)}`);
       const response = await this.managerMetrics.getOutboundByteTransfer({hours: 30 * 24});
@@ -612,7 +604,7 @@ export class ShadowsocksManagerService {
     }
   }
 
-  public getShareMetrics(req: RequestType, res: ResponseType, next: restify.Next): void {
+  getShareMetrics(req: RequestType, res: ResponseType, next: restify.Next): void {
     logging.debug(`getShareMetrics request ${JSON.stringify(req.params)}`);
     const response = {metricsEnabled: this.metricsPublisher.isSharingEnabled()};
     res.send(HttpSuccess.OK, response);
@@ -620,7 +612,7 @@ export class ShadowsocksManagerService {
     next();
   }
 
-  public setShareMetrics(req: RequestType, res: ResponseType, next: restify.Next): void {
+  setShareMetrics(req: RequestType, res: ResponseType, next: restify.Next): void {
     logging.debug(`setShareMetrics request ${JSON.stringify(req.params)}`);
     const metricsEnabled = req.params.metricsEnabled;
     if (metricsEnabled === undefined || metricsEnabled === null) {
@@ -647,7 +639,7 @@ export class ShadowsocksManagerService {
     next();
   }
 
-  public enableAsnMetrics(req: RequestType, res: ResponseType, next: restify.Next): void {
+  enableAsnMetrics(req: RequestType, res: ResponseType, next: restify.Next): void {
     try {
       logging.debug(`enableAsnMetrics request ${JSON.stringify(req.params)}`);
       const asnMetricsEnabled = req.params.asnMetricsEnabled;

src/shadowbox/server/server_access_key.spec.ts

@@ -806,28 +806,28 @@ class RepoBuilder {
   private prometheusClient_ = new FakePrometheusClient({});
   private defaultDataLimit_;
 
-  public port(port: number): RepoBuilder {
+  port(port: number): RepoBuilder {
     this.port_ = port;
     return this;
   }
-  public keyConfig(keyConfig: InMemoryConfig<AccessKeyConfigJson>): RepoBuilder {
+  keyConfig(keyConfig: InMemoryConfig<AccessKeyConfigJson>): RepoBuilder {
     this.keyConfig_ = keyConfig;
     return this;
   }
-  public shadowsocksServer(shadowsocksServer: FakeShadowsocksServer): RepoBuilder {
+  shadowsocksServer(shadowsocksServer: FakeShadowsocksServer): RepoBuilder {
     this.shadowsocksServer_ = shadowsocksServer;
     return this;
   }
-  public prometheusClient(prometheusClient: FakePrometheusClient): RepoBuilder {
+  prometheusClient(prometheusClient: FakePrometheusClient): RepoBuilder {
     this.prometheusClient_ = prometheusClient;
     return this;
   }
-  public defaultDataLimit(limit: DataLimit): RepoBuilder {
+  defaultDataLimit(limit: DataLimit): RepoBuilder {
     this.defaultDataLimit_ = limit;
     return this;
   }
 
-  public build(): ServerAccessKeyRepository {
+  build(): ServerAccessKeyRepository {
     return new ServerAccessKeyRepository(
       this.port_,
       'hostname',

src/shadowbox/server/server_access_key.ts

@@ -50,7 +50,7 @@ export interface AccessKeyConfigJson {
 
 // AccessKey implementation with write access enabled on properties that may change.
 class ServerAccessKey implements AccessKey {
-  public isOverDataLimit = false;
+  isOverDataLimit = false;
   constructor(
     readonly id: AccessKeyId,
     public name: string,

src/shadowbox/server/shared_metrics.spec.ts

@@ -244,8 +244,8 @@ describe('OutlineSharedMetricsPublisher', () => {
 });
 
 class FakeMetricsCollector implements MetricsCollectorClient {
-  public collectedServerUsageReport: HourlyServerMetricsReportJson;
+  collectedServerUsageReport: HourlyServerMetricsReportJson;
-  public collectedFeatureMetricsReport: DailyFeatureMetricsReportJson;
+  collectedFeatureMetricsReport: DailyFeatureMetricsReportJson;
 
   async collectServerUsageMetrics(report) {
     this.collectedServerUsageReport = report;
@@ -257,7 +257,7 @@ class FakeMetricsCollector implements MetricsCollectorClient {
 }
 
 class ManualUsageMetrics implements UsageMetrics {
-  public countryUsage = [] as CountryUsage[];
+  countryUsage = [] as CountryUsage[];
 
   getCountryUsage(): Promise<CountryUsage[]> {
     return Promise.resolve(this.countryUsage);