Repository: elm-tooling/elm-language-server
Pull request #579 (2021-04-25 09:59:55)

Change default for singleFieldRecord rule to be false
We now default to this to off, as we had multiple people point out that
they're no fans of this rule. And I think we agree, as we
just inherrited this from elm-analyze.


Patch:
diff --git a/src/index.ts b/src/index.ts
index f7e195a..948e0c7 100644
--- a/src/index.ts
+++ b/src/index.ts
@@ -14,6 +14,10 @@ import Parser from "web-tree-sitter";
 import { getCancellationStrategyFromArgv } from "./cancellation";
 import { CapabilityCalculator } from "./capabilityCalculator";
 import { ASTProvider } from "./providers";
+import {
+  ElmAnalyseJsonService,
+  IElmAnalyseJsonService,
+} from "./providers/diagnostics/elmAnalyseJsonService";
 import { ILanguageServer } from "./server";
 import { DocumentEvents } from "./util/documentEvents";
 import { Settings } from "./util/settings";
@@ -40,6 +44,10 @@ container.register<Connection>("Connection", {
 container.registerSingleton<Parser>("Parser", Parser);
 
 container.registerSingleton("DocumentEvents", DocumentEvents);
+container.registerSingleton<IElmAnalyseJsonService>(
+  "ElmAnalyseJsonService",
+  ElmAnalyseJsonService,
+);
 container.register(TextDocumentEvents, {
   useValue: new TextDocumentEvents(),
 });
diff --git a/src/providers/diagnostics/elmAnalyseJsonService.ts b/src/providers/diagnostics/elmAnalyseJsonService.ts
new file mode 100644
index 0000000..eb0f8d2
--- /dev/null
+++ b/src/providers/diagnostics/elmAnalyseJsonService.ts
@@ -0,0 +1,75 @@
+import path from "path";
+import { container, injectable } from "tsyringe";
+import { Connection } from "vscode-languageserver";
+
+export interface IElmAnalyseJson {
+  checks?: {
+    BooleanCase?: boolean;
+    DebugLog?: boolean; // We don't support this
+    DebugTodo?: boolean; // We don't support this
+    DropConcatOfLists?: boolean;
+    DropConsOfItemAndList?: boolean;
+    DuplicateImport?: boolean; // We don't support this as elm-format will fix this
+    DuplicateImportedVariable?: boolean; // We don't support this as elm-format will fix this
+    ExposeAll?: boolean; // We don't support this
+    FileLoadFailed?: boolean; // We don't support this as it makes no sense for us
+    NoUncurriedPrefix?: boolean;
+    FunctionInLet?: boolean; // We don't support this
+    ImportAll?: boolean; // We don't support this
+    MapNothingToNothing?: boolean;
+    MultiLineRecordFormatting?: boolean; // We don't support this
+    NoTopLevelSignature?: boolean; // We don't support this as we get it via type inference already
+    SingleFieldRecord?: boolean;
+    TriggerWords?: string[]; // We don't support this
+    UnnecessaryListConcat?: boolean;
+    UnnecessaryParens?: boolean; // We don't support this as elm-format will fix these anyway
+    UnnecessaryPortModule?: boolean;
+    UnusedImport?: boolean;
+    UnusedImportAlias?: boolean;
+    UnusedImportedVariable?: boolean;
+    UnusedPatternVariable?: boolean;
+    UnusedTopLevel?: boolean;
+    UnusedTypeAlias?: boolean;
+    UnusedValueConstructor?: boolean;
+    UnusedVariable?: boolean;
+    UseConsOverConcat?: boolean;
+  };
+  excludedPaths?: string[];
+}
+
+export interface IElmAnalyseJsonService {
+  getElmAnalyseJson(workspacePath: string): IElmAnalyseJson;
+}
+
+@injectable()
+export class ElmAnalyseJsonService implements IElmAnalyseJsonService {
+  private connection: Connection;
+  private elmAnalyseJson = new Map<string, IElmAnalyseJson>();
+
+  constructor() {
+    this.connection = container.resolve<Connection>("Connection");
+  }
+
+  public getElmAnalyseJson(workspacePath: string): IElmAnalyseJson {
+    const cached = this.elmAnalyseJson.get(workspacePath);
+
+    if (cached) {
+      return cached;
+    }
+
+    let elmAnalyseJson = {};
+    try {
+      elmAnalyseJson = require(path.join(
+        workspacePath,
+        "elm-analyse.json",
+      )) as IElmAnalyseJson;
+    } catch {
+      this.connection.console.info(
+        "No elm-analyse.json found, enabling all diagnostic checks.",
+      );
+    }
+
+    this.elmAnalyseJson.set(workspacePath, elmAnalyseJson);
+    return elmAnalyseJson;
+  }
+}
diff --git a/src/providers/diagnostics/elmLsDiagnostics.ts b/src/providers/diagnostics/elmLsDiagnostics.ts
index 271fd26..58aa328 100644
--- a/src/providers/diagnostics/elmLsDiagnostics.ts
+++ b/src/providers/diagnostics/elmLsDiagnostics.ts
@@ -24,46 +24,13 @@ import { Utils } from "../../util/utils";
 import { IDiagnostic } from "./diagnosticsProvider";
 import * as path from "path";
 import { SyntaxNodeMap } from "../../compiler/utils/syntaxNodeMap";
+import { IElmAnalyseJsonService } from "./elmAnalyseJsonService";
 
-interface IElmAnalyseJson {
-  checks?: {
-    BooleanCase?: boolean;
-    DebugLog?: boolean; // We don't support this
-    DebugTodo?: boolean; // We don't support this
-    DropConcatOfLists?: boolean;
-    DropConsOfItemAndList?: boolean;
-    DuplicateImport?: boolean; // We don't support this as elm-format will fix this
-    DuplicateImportedVariable?: boolean; // We don't support this as elm-format will fix this
-    ExposeAll?: boolean; // We don't support this
-    FileLoadFailed?: boolean; // We don't support this as it makes no sense for us
-    NoUncurriedPrefix?: boolean;
-    FunctionInLet?: boolean; // We don't support this
-    ImportAll?: boolean; // We don't support this
-    MapNothingToNothing?: boolean;
-    MultiLineRecordFormatting?: boolean; // We don't support this
-    NoTopLevelSignature?: boolean; // We don't support this as we get it via type inference already
-    SingleFieldRecord?: boolean;
-    TriggerWords?: string[]; // We don't support this
-    UnnecessaryListConcat?: boolean;
-    UnnecessaryParens?: boolean; // We don't support this as elm-format will fix these anyway
-    UnnecessaryPortModule?: boolean;
-    UnusedImport?: boolean;
-    UnusedImportAlias?: boolean;
-    UnusedImportedVariable?: boolean;
-    UnusedPatternVariable?: boolean;
-    UnusedTopLevel?: boolean;
-    UnusedTypeAlias?: boolean;
-    UnusedValueConstructor?: boolean;
-    UnusedVariable?: boolean;
-    UseConsOverConcat?: boolean;
-  };
-  excludedPaths?: string[];
-}
 export class ElmLsDiagnostics {
   private language: Language;
   private elmWorkspaceMatcher: ElmWorkspaceMatcher<URI>;
   private connection: Connection;
-  private elmAnalyseJson = new Map<string, IElmAnalyseJson>();
+  private elmAnalyseJsonService: IElmAnalyseJsonService;
 
   private readonly exposedValuesAndTypesQuery: Query;
   private readonly exposedValueAndTypeUsagesQuery: Query;
@@ -91,6 +58,9 @@ export class ElmLsDiagnostics {
     this.language = container.resolve<Parser>("Parser").getLanguage();
     this.elmWorkspaceMatcher = new ElmWorkspaceMatcher((uri) => uri);
     this.connection = container.resolve<Connection>("Connection");
+    this.elmAnalyseJsonService = container.resolve<IElmAnalyseJsonService>(
+      "ElmAnalyseJsonService",
+    );
 
     this.exposedValuesAndTypesQuery = this.language.query(
       `
@@ -422,7 +392,9 @@ export class ElmLsDiagnostics {
     sourceFile: ISourceFile,
     program: IProgram,
   ): IDiagnostic[] => {
-    const elmAnalyseJson = this.getElmAnalyseJson(program.getRootPath().fsPath);
+    const elmAnalyseJson = this.elmAnalyseJsonService.getElmAnalyseJson(
+      program.getRootPath().fsPath,
+    );
     const tree = sourceFile.tree;
     const uri = sourceFile.uri;
     const rootPath = program.getRootPath().fsPath;
@@ -472,9 +444,9 @@ export class ElmLsDiagnostics {
         ...(elmAnalyseJson.checks?.UseConsOverConcat === false
           ? []
           : this.getUseConsOverConcatDiagnostics(tree)),
-        ...(elmAnalyseJson.checks?.SingleFieldRecord === false
-          ? []
-          : this.getSingleFieldRecordDiagnostics(tree, uri, program)),
+        ...(elmAnalyseJson.checks?.SingleFieldRecord
+          ? this.getSingleFieldRecordDiagnostics(tree, uri, program)
+          : []),
         ...(elmAnalyseJson.checks?.UnnecessaryListConcat === false
           ? []
           : this.getUnnecessaryListConcatDiagnostics(tree)),
@@ -497,29 +469,6 @@ export class ElmLsDiagnostics {
     return [];
   };
 
-  private getElmAnalyseJson(workspacePath: string): IElmAnalyseJson {
-    const cached = this.elmAnalyseJson.get(workspacePath);
-
-    if (cached) {
-      return cached;
-    }
-
-    let elmAnalyseJson = {};
-    try {
-      elmAnalyseJson = require(path.join(
-        workspacePath,
-        "elm-analyse.json",
-      )) as IElmAnalyseJson;
-    } catch {
-      this.connection.console.info(
-        "No elm-analyse.json found, enabling all diagnostic checks.",
-      );
-    }
-
-    this.elmAnalyseJson.set(workspacePath, elmAnalyseJson);
-    return elmAnalyseJson;
-  }
-
   private getUnusedImportDiagnostics(tree: Tree): IDiagnostic[] {
     const diagnostics: IDiagnostic[] = [];
 


Test patch:
diff --git a/test/jest.setup.ts b/test/jest.setup.ts
index 496ea70..bb5cf03 100644
--- a/test/jest.setup.ts
+++ b/test/jest.setup.ts
@@ -4,6 +4,10 @@ import { Connection } from "vscode-languageserver";
 import { mockDeep } from "jest-mock-extended";
 import { Settings } from "../src/util/settings";
 import { DocumentEvents } from "../src/util/documentEvents";
+import {
+  IElmAnalyseJsonService,
+  IElmAnalyseJson,
+} from "../src/providers/diagnostics/elmAnalyseJsonService";
 
 container.register("Connection", { useValue: mockDeep<Connection>() });
 container.register("ElmWorkspaces", { useValue: [] });
@@ -17,3 +21,11 @@ container.register("ClientSettings", {
   useValue: {},
 });
 container.registerSingleton("DocumentEvents", DocumentEvents);
+container.registerSingleton<IElmAnalyseJsonService>(
+  "ElmAnalyseJsonService",
+  class ElmAnalyseJsonHelperFixed implements IElmAnalyseJsonService {
+    public getElmAnalyseJson(workspacePath: string): IElmAnalyseJson {
+      return { checks: { SingleFieldRecord: true } };
+    }
+  },
+);