feat(eslint-config): Update constructor access modifier eslint rule t…

…o require explicit modifiers (#13134) Updates rule to match discussion outcome in [this discussion](#9502 (comment)). Rule documentation: https://typescript-eslint.io/rules/explicit-member-accessibility/ Also fixes violations in packages using the `strict` config ### Breaking Change Note that this is a breaking change with respect to the above rule. We are changing error-level enforcement of the policy to the opposite rule, so code changes will be required. This PR pre-fixes the violations existing at this point in time, but any changes between now and when the new package is published and adopted will need to be fixed.
microsoft · Dec 5, 2022 · faa9111 · faa9111
1 parent 1b00075
commit faa9111
Show file tree

Hide file tree

Showing 20 changed files with 108 additions and 16 deletions.
diff --git a/azure/packages/azure-client/.eslintrc.js b/azure/packages/azure-client/.eslintrc.js
@@ -31,6 +31,21 @@ module.exports = {
         // Ensure jsdoc/tsdoc comments contain a main description component
         // (disallows empty comments / only tags).
         "jsdoc/require-description": ["error", { checkConstructors: false }],
+
+        // TODO: remove once dependency on base config has been updated.
+        "@typescript-eslint/explicit-member-accessibility": [
+            "error",
+            {
+                accessibility: "explicit",
+                overrides: {
+                    accessors: "explicit",
+                    constructors: "explicit",
+                    methods: "explicit",
+                    properties: "explicit",
+                    parameterProperties: "explicit",
+                },
+            },
+        ],
     },
     overrides: [
         {

diff --git a/azure/packages/azure-client/src/AzureClient.ts b/azure/packages/azure-client/src/AzureClient.ts
@@ -56,7 +56,7 @@ export class AzureClient {
      * Creates a new client instance using configuration parameters.
      * @param props - Properties for initializing a new AzureClient instance
      */
-    constructor(private readonly props: AzureClientProps) {
+    public constructor(private readonly props: AzureClientProps) {
         // remove trailing slash from URL if any
         props.connection.endpoint = props.connection.endpoint.replace(/\/$/, "");
         this.urlResolver = new AzureUrlResolver();

diff --git a/azure/packages/azure-client/src/AzureFunctionTokenProvider.ts b/azure/packages/azure-client/src/AzureFunctionTokenProvider.ts
@@ -18,7 +18,7 @@ export class AzureFunctionTokenProvider implements ITokenProvider {
      * @param azFunctionUrl - URL to Azure Function endpoint
      * @param user - User object
      */
-    constructor(
+    public constructor(
         private readonly azFunctionUrl: string,
         private readonly user?: Pick<AzureMember, "userId" | "userName" | "additionalDetails">,
     ) {}

diff --git a/azure/packages/azure-client/src/AzureUrlResolver.ts b/azure/packages/azure-client/src/AzureUrlResolver.ts
@@ -18,7 +18,7 @@ import {
  * providers that fulfill the {@link @fluidframework/routerlicious-driver#ITokenProvider} interface.
  */
 export class AzureUrlResolver implements IUrlResolver {
-    constructor() {}
+    public constructor() {}
 
     public async resolve(request: IRequest): Promise<IFluidResolvedUrl> {
         const { ordererUrl, storageUrl, tenantId, containerId } = decodeAzureUrl(request.url);

diff --git a/azure/packages/azure-local-service/.eslintrc.js b/azure/packages/azure-local-service/.eslintrc.js
@@ -31,6 +31,21 @@ module.exports = {
         // Ensure jsdoc/tsdoc comments contain a main description component
         // (disallows empty comments / only tags).
         "jsdoc/require-description": ["error", { checkConstructors: false }],
+
+        // TODO: remove once dependency on base config has been updated.
+        "@typescript-eslint/explicit-member-accessibility": [
+            "error",
+            {
+                accessibility: "explicit",
+                overrides: {
+                    accessors: "explicit",
+                    constructors: "explicit",
+                    methods: "explicit",
+                    properties: "explicit",
+                    parameterProperties: "explicit",
+                },
+            },
+        ],
     },
     overrides: [
         {

diff --git a/azure/packages/azure-service-utils/.eslintrc.js b/azure/packages/azure-service-utils/.eslintrc.js
@@ -34,6 +34,21 @@ module.exports = {
         // Ensure jsdoc/tsdoc comments contain a main description component
         // (disallows empty comments / only tags).
         "jsdoc/require-description": ["error", { checkConstructors: false }],
+
+        // TODO: remove once dependency on base config has been updated.
+        "@typescript-eslint/explicit-member-accessibility": [
+            "error",
+            {
+                accessibility: "explicit",
+                overrides: {
+                    accessors: "explicit",
+                    constructors: "explicit",
+                    methods: "explicit",
+                    properties: "explicit",
+                    parameterProperties: "explicit",
+                },
+            },
+        ],
     },
     overrides: [
         {

diff --git a/common/build/eslint-config-fluid/printed-configs/strict.json b/common/build/eslint-config-fluid/printed-configs/strict.json
@@ -106,7 +106,7 @@
                 "accessibility": "explicit",
                 "overrides": {
                     "accessors": "explicit",
-                    "constructors": "no-public",
+                    "constructors": "explicit",
                     "methods": "explicit",
                     "parameterProperties": "explicit",
                     "properties": "explicit"

diff --git a/common/build/eslint-config-fluid/strict.js b/common/build/eslint-config-fluid/strict.js
@@ -17,7 +17,7 @@ module.exports = {
                         accessibility: "explicit",
                         overrides: {
                             accessors: "explicit",
-                            constructors: "no-public",
+                            constructors: "explicit",
                             methods: "explicit",
                             properties: "explicit",
                             parameterProperties: "explicit",

diff --git a/examples/hosts/app-integration/external-data/.eslintrc.js b/examples/hosts/app-integration/external-data/.eslintrc.js
@@ -10,5 +10,20 @@ module.exports = {
             "prettier"
         ),
     ],
-    rules: {},
+    rules: {
+        // TODO: remove once dependency on base config has been updated.
+        "@typescript-eslint/explicit-member-accessibility": [
+            "error",
+            {
+                accessibility: "explicit",
+                overrides: {
+                    accessors: "explicit",
+                    constructors: "explicit",
+                    methods: "explicit",
+                    properties: "explicit",
+                    parameterProperties: "explicit",
+                },
+            },
+        ],
+    },
 };
diff --git a/examples/hosts/app-integration/external-data/src/externalData.ts b/examples/hosts/app-integration/external-data/src/externalData.ts
@@ -63,7 +63,7 @@ export interface IExternalDataSourceEvents extends IEvent {
  * on the server-side bot approach.  But using localStorage is probably good enough for the broadcast signal portion.
  */
 export class ExternalDataSource extends TypedEventEmitter<IExternalDataSourceEvents> {
-    constructor() {
+    public constructor() {
         super();
         if (window.localStorage.getItem(localStorageKey) === null) {
             this.debugResetData();

diff --git a/examples/hosts/app-integration/external-data/src/model/appModel.ts b/examples/hosts/app-integration/external-data/src/model/appModel.ts
@@ -13,7 +13,7 @@ import type { IAppModel, IAppModelEvents, ITaskList } from "../modelInterfaces";
  * responsibilities and functionality.
  */
 export class AppModel extends TypedEventEmitter<IAppModelEvents> implements IAppModel {
-    constructor(public readonly taskList: ITaskList, container: IContainer) {
+    public constructor(public readonly taskList: ITaskList, container: IContainer) {
         super();
     }
 }
diff --git a/examples/hosts/app-integration/external-data/src/model/containerCode.ts b/examples/hosts/app-integration/external-data/src/model/containerCode.ts
@@ -27,7 +27,7 @@ const SignalType = {
  * {@inheritDoc ModelContainerRuntimeFactory}
  */
 export class TaskListContainerRuntimeFactory extends ModelContainerRuntimeFactory<IAppModel> {
-    constructor() {
+    public constructor() {
         super(
             new Map([TaskListInstantiationFactory.registryEntry]), // registryEntries
         );

diff --git a/examples/hosts/app-integration/external-data/src/model/taskList.ts b/examples/hosts/app-integration/external-data/src/model/taskList.ts
@@ -30,7 +30,7 @@ class Task extends TypedEventEmitter<ITaskEvents> implements ITask {
     public set priority(newValue: number) {
         this._priority.set(newValue);
     }
-    constructor(
+    public constructor(
         private readonly _id: string,
         private readonly _name: SharedString,
         private readonly _priority: ISharedCell<number>

diff --git a/packages/dds/quorum/.eslintrc.js b/packages/dds/quorum/.eslintrc.js
@@ -10,5 +10,20 @@ module.exports = {
     ],
     "parserOptions": {
         "project": ["./tsconfig.json", "./src/test/tsconfig.json"]
-    },
+    }, "rules": {
+        // TODO: remove once dependency on base config has been updated.
+        "@typescript-eslint/explicit-member-accessibility": [
+            "error",
+            {
+                accessibility: "explicit",
+                overrides: {
+                    accessors: "explicit",
+                    constructors: "explicit",
+                    methods: "explicit",
+                    properties: "explicit",
+                    parameterProperties: "explicit",
+                },
+            },
+        ],
+    }
 }
diff --git a/packages/dds/quorum/src/quorum.ts b/packages/dds/quorum/src/quorum.ts
@@ -190,7 +190,6 @@ export class Quorum<T = unknown> extends SharedObject<IQuorumEvents> implements
      * @param runtime - data store runtime the quorum belongs to
      * @param id - optional name of the quorum
      */
-    // eslint-disable-next-line @typescript-eslint/explicit-member-accessibility
     public constructor(id: string, runtime: IFluidDataStoreRuntime, attributes: IChannelAttributes) {
         super(id, runtime, attributes, "fluid_quorum_");
 

diff --git a/tools/api-markdown-documenter/.eslintrc.js b/tools/api-markdown-documenter/.eslintrc.js
@@ -27,5 +27,20 @@ module.exports = {
 
         // This package is exclusively used in a Node.js context
         "import/no-nodejs-modules": "off",
+
+        // TODO: remove once dependency on base config has been updated.
+        "@typescript-eslint/explicit-member-accessibility": [
+            "error",
+            {
+                accessibility: "explicit",
+                overrides: {
+                    accessors: "explicit",
+                    constructors: "explicit",
+                    methods: "explicit",
+                    properties: "explicit",
+                    parameterProperties: "explicit",
+                },
+            },
+        ],
     },
 };
diff --git a/tools/api-markdown-documenter/src/MarkdownEmitter.ts b/tools/api-markdown-documenter/src/MarkdownEmitter.ts
@@ -111,7 +111,10 @@ export class MarkdownEmitter extends BaseMarkdownEmitter {
      * @param apiModel - See {@link MarkdownEmitter.apiModel}.
      * @param generateFrontMatter - See {@link MarkdownEmitter.generateFrontMatter}.
      */
-    constructor(apiModel: ApiModel, generateFrontMatter?: (contextApiItem: ApiItem) => string) {
+    public constructor(
+        apiModel: ApiModel,
+        generateFrontMatter?: (contextApiItem: ApiItem) => string,
+    ) {
         super();
         this.apiModel = apiModel;
         this.generateFrontMatter = generateFrontMatter;

diff --git a/tools/api-markdown-documenter/src/doc-nodes/DocAlert.ts b/tools/api-markdown-documenter/src/doc-nodes/DocAlert.ts
@@ -50,7 +50,7 @@ export class DocAlert extends DocNode {
      */
     public readonly content: DocNode;
 
-    constructor(parameters: IDocAlertParameters, content: DocNode) {
+    public constructor(parameters: IDocAlertParameters, content: DocNode) {
         super(parameters);
 
         this.type = parameters.type;

diff --git a/tools/api-markdown-documenter/src/doc-nodes/DocHeading.ts b/tools/api-markdown-documenter/src/doc-nodes/DocHeading.ts
@@ -34,7 +34,7 @@ export class DocHeading extends DocNode {
      */
     public readonly id?: string;
 
-    constructor(parameters: IDocHeadingParameters) {
+    public constructor(parameters: IDocHeadingParameters) {
         super(parameters);
 
         this.title = parameters.title;

diff --git a/tools/api-markdown-documenter/src/doc-nodes/DocList.ts b/tools/api-markdown-documenter/src/doc-nodes/DocList.ts
@@ -35,7 +35,7 @@ export class DocList extends DocNodeContainer {
      */
     public readonly listKind: ListKind;
 
-    constructor(parameters: IDocListParameters, childNodes?: readonly DocNode[]) {
+    public constructor(parameters: IDocListParameters, childNodes?: readonly DocNode[]) {
         super(parameters, childNodes);
 
         this.listKind = parameters.listKind ?? ListKind.Unordered;