Formatter: Disable formatter in default config (marcoroth#944)

This pull request changes the default configuration to not have the
formatter enabled by default. Since the formatter is still experimental
people should be opting in to use it.

Since the default configuration had the formatter enabled by default
this meant that the language server started formatting on save by
default as well, which is not ideal.

This pull request addresses this, so that people have to opt-in and
enable the formatter in teh `.herb.yml` configuration file.
Additionally, it splits up the fix and format concerns in the language
server, so that the `editor.formatOnSave` setting in VS Code is also
properly respected.

Resolves marcoroth#910
Resolves marcoroth#911

Author: marcoroth

javascript/packages/config/src/config.ts

@@ -139,10 +139,10 @@ export class Config {
 
   /**
    * Check if the formatter is enabled.
-   * @returns true if formatter is enabled (default), false if explicitly disabled
+   * @returns true if formatter is explicitly enabled, false otherwise (default)
    */
   public get isFormatterEnabled(): boolean {
-    return this.config.formatter?.enabled ?? Config.getDefaultConfig().formatter?.enabled ?? true
+    return this.config.formatter?.enabled ?? Config.getDefaultConfig().formatter?.enabled ?? false
   }
 
   /**
@@ -1096,7 +1096,7 @@ export class Config {
         rules: {}
       },
       formatter: {
-        enabled: true,
+        enabled: false,
         indentWidth: 2,
         maxLineLength: 80
       }

javascript/packages/config/test/config.test.ts

@@ -379,10 +379,10 @@ describe("@herb-tools/config", () => {
       expect(config.isFormatterEnabled).toBe(false)
     })
 
-    test("isFormatterEnabled returns true by default", () => {
+    test("isFormatterEnabled returns false by default", () => {
       const config = Config.fromObject({}, { projectPath: testDir })
 
-      expect(config.isFormatterEnabled).toBe(true)
+      expect(config.isFormatterEnabled).toBe(false)
     })
 
     test("isRuleDisabled returns true when rule is disabled", () => {

javascript/packages/formatter/src/cli.ts

@@ -171,17 +171,18 @@ export class CLI {
       }
 
       const config = await Config.loadForCLI(configFile || startPath, version)
+      const hasConfigFile = Config.exists(config.projectPath)
       const formatterConfig = config.formatter || {}
 
-      if (formatterConfig.enabled === false && !isForceMode) {
+      if (hasConfigFile && formatterConfig.enabled === false && !isForceMode) {
         console.log("Formatter is disabled in .herb.yml configuration.")
         console.log("To enable formatting, set formatter.enabled: true in .herb.yml")
         console.log("Or use --force to format anyway.")
 
         process.exit(0)
       }
 
-      if (isForceMode && formatterConfig.enabled === false) {
+      if (isForceMode && hasConfigFile && formatterConfig.enabled === false) {
         console.error("⚠️  Forcing formatter run (disabled in .herb.yml)")
         console.error()
       }

javascript/packages/language-server/src/document_save_service.ts

@@ -18,12 +18,31 @@ export class DocumentSaveService {
     this.formattingService = formattingService
   }
 
+  /**
+   * Apply only autofix edits on save.
+   * Called by willSaveWaitUntil - formatting is handled separately by editor.formatOnSave
+   */
+  async applyFixes(document: TextDocument): Promise<TextEdit[]> {
+    const settings = await this.settings.getDocumentSettings(document.uri)
+    const fixOnSave = settings?.linter?.fixOnSave !== false
+
+    this.connection.console.log(`[DocumentSave] applyFixes fixOnSave=${fixOnSave}`)
+
+    if (!fixOnSave) return []
+
+    return this.autofixService.autofix(document)
+  }
+
+  /**
+   * Apply autofix and formatting.
+   * Called by onDocumentFormatting (manual format or editor.formatOnSave)
+   */
   async applyFixesAndFormatting(document: TextDocument, reason: TextDocumentSaveReason): Promise<TextEdit[]> {
     const settings = await this.settings.getDocumentSettings(document.uri)
     const fixOnSave = settings?.linter?.fixOnSave !== false
     const formatterEnabled = settings?.formatter?.enabled ?? false
 
-    this.connection.console.log(`[DocumentSave] fixOnSave=${fixOnSave}, formatterEnabled=${formatterEnabled}`)
+    this.connection.console.log(`[DocumentSave] applyFixesAndFormatting fixOnSave=${fixOnSave}, formatterEnabled=${formatterEnabled}`)
 
     let autofixEdits: TextEdit[] = []
 

javascript/packages/language-server/src/formatting_service.ts

@@ -210,6 +210,9 @@ export class FormattingService {
     if (filePath.endsWith('.herb.yml')) return false
     if (!this.config) return true
 
+    const hasConfigFile = Config.exists(this.config.projectPath)
+    if (!hasConfigFile) return true
+
     const relativePath = filePath.replace('file://', '').replace(this.project.projectPath + '/', '')
 
     return this.config.isFormatterEnabledForPath(relativePath)

javascript/packages/language-server/src/server.ts

@@ -12,15 +12,12 @@ import {
   DocumentRangeFormattingParams,
   CodeActionParams,
   CodeActionKind,
-  TextEdit,
 } from "vscode-languageserver/node"
 
 import { Service } from "./service"
 import { PersonalHerbSettings } from "./settings"
 import { Config } from "@herb-tools/config"
 
-import type { TextDocument } from "vscode-languageserver-textdocument"
-
 export class Server {
   private service!: Service
   private connection: Connection
@@ -37,7 +34,7 @@ export class Server {
       await this.service.init()
 
       this.service.documentService.documents.onWillSaveWaitUntil(async (event) => {
-        return this.service.documentSaveService.applyFixesAndFormatting(event.document, event.reason)
+        return this.service.documentSaveService.applyFixes(event.document)
       })
 
       const result: InitializeResult = {

javascript/packages/language-server/src/settings.ts

@@ -87,8 +87,9 @@ export class Settings {
   // TODO: ideally we can just use Config all the way through
   private mergeSettings(userSettings: PersonalHerbSettings | null, projectConfig?: Config): PersonalHerbSettings {
     const settings = userSettings || this.defaultSettings
+    const hasConfigFile = projectConfig ? Config.exists(projectConfig.projectPath) : false
 
-    if (!projectConfig) {
+    if (!projectConfig || !hasConfigFile) {
       return {
         trace: settings.trace,
         linter: {

javascript/packages/language-server/test/document_save_service.test.ts

@@ -274,7 +274,7 @@ describe('DocumentSaveService', () => {
         )
 
         expect(connection.console.log).toHaveBeenCalledWith(
-          '[DocumentSave] fixOnSave=true, formatterEnabled=true'
+          '[DocumentSave] applyFixesAndFormatting fixOnSave=true, formatterEnabled=true'
         )
       })
     })