Project

General

Profile

Actions

Architecture #21741

closed

Auto-format scala code

Added by François ARMAND over 1 year ago. Updated 20 days ago.

Status:
Released
Priority:
N/A
Category:
Architecture - Code maintenance
Target version:
Effort required:
Name check:
To do
Fix check:
Checked
Regression:
No

Description

The default code formater is scalafmt https://scalameta.org/scalafmt/docs/configuration.html

It's integrated with intellij and there is several maven plugin for it (spotless seems well maintained and documented: https://github.com/diffplug/spotless/tree/main/plugin-maven

At first, we will try to keep scalafmt configuration simple. ZIO config can be used as a base.

Change must be done in all supported branches to avoid upmerge conflict (so, 6.2 to master).

We can even make the corresponding commit be ignored with git blame: https://akrabat.com/ignoring-revisions-with-git-blame/#store-the-ignored-commits-in-a-file


Subtasks 12 (0 open12 closed)

Architecture #21995: Auto-format scala code (7.2)ReleasedAlexis MoussetActions
Architecture #21996: Add Scala format tests on pull requestsReleasedFrançois ARMANDActions
Rudder plugins - Architecture #22002: Add Scala format tests on pull requests - pluginsRejectedFrançois ARMANDActions
Architecture #21998: Auto-format scala code (master)ReleasedAlexis MoussetActions
Rudder plugins - Architecture #21999: Auto-format scala code (plugins branch 7.1)ReleasedAlexis MoussetActions
Rudder plugins - Bug #22026: Missing auto format on plugin-private-commonReleasedAlexis MoussetActions
Rudder plugins - Architecture #22000: Auto-format scala code (plugins branch 7.2)ReleasedVincent MEMBRÉActions
Rudder plugins - Architecture #22001: Auto-format scala code (plugins branch master)ReleasedVincent MEMBRÉActions
Rudder plugins - Architecture #22005: Auto-format scala code (plugins private branch 7.1)ReleasedAlexis MoussetActions
Rudder plugins - Architecture #22006: Add spotless (plugins-private)ReleasedAlexis MoussetActions
Rudder plugins - Architecture #22007: Auto-format scala code (plugins private branch 7.2)ReleasedAlexis MoussetActions
Rudder plugins - Architecture #22009: Auto-format scala code (plugins private branch master)ReleasedAlexis MoussetActions

Related issues 1 (1 open0 closed)

Related to Rudder plugins - Bug #22004: Spotless must not be exec in plugin parentPending releaseAlexis MoussetActions
Actions #1

Updated by François ARMAND over 1 year ago

  • Status changed from New to In progress
  • Assignee set to François ARMAND
Actions #2

Updated by François ARMAND over 1 year ago

  • Status changed from In progress to Pending technical review
  • Assignee changed from François ARMAND to Vincent MEMBRÉ
  • Pull Request set to https://github.com/Normation/rudder/pull/4473
Actions #3

Updated by François ARMAND over 1 year ago

  • Status changed from Pending technical review to In progress
  • Assignee changed from Vincent MEMBRÉ to François ARMAND
Actions #4

Updated by François ARMAND over 1 year ago

.scalafmt.conf that seems to be ok:

version = "3.5.9" 
maxColumn = 130
runner.debug = true
runner.dialect = scala213

align.preset = most
align.multiline = true
align.tokens."+" = [
  {code = ":", owner = ".*"},
]
align.arrowEnumeratorGenerator = true
align.allowOverflow = true

docstrings.style = Asterisk
docstrings.wrapMaxColumn = 100
docstrings.blankFirstLine = yes
lineEndings = preserve
includeCurlyBraceInSelectChains = false
danglingParentheses.preset = true
optIn.annotationNewlines = true
binPack.parentConstructors = Always
indentOperator.exemptScope = all
indentOperator.excludeRegex = "^.*$" 
assumeStandardLibraryStripMargin = true

// try to have condensed lambda, but when multiline is needed,
// keep param with the open '{' :
// foo.map { x =>
//  ....
newlines.alwaysBeforeMultilineDef = false
newlines.beforeCurlyLambdaParams = multilineWithCaseOnly
newlines.afterCurlyLambdaParams = squash

rewrite.rules = [RedundantBraces, SortModifiers, PreferCurlyFors, Imports]
rewrite.redundantBraces.defnBodies = none // keep around def body
rewrite.redundantBraces.generalExpressions = false // keep {} around while/etc
rewrite.redundantBraces.stringInterpolation = false // keep {} in string interpol
// use () in place of {} for lambda when possible
rewrite.redundantBraces.parensForOneLineApply = true

// remove {} is there is only one line in the block
rewrite.redundantBraces.maxBreaks = 2
rewrite.insertBraces.minLines = 2
rewrite.insertBraces.allBlocks = true
// one import per line appart when renaming imported element
rewrite.imports.expand = true
rewrite.imports.sort = scalastyle

rewriteTokens = {
  "⇒": "=>" 
  "→": "->" 
  "←": "<-" 
}

Change in pom.xml to enable the formatter plugin. By default, it checks correctness on verify phase, ie between package and install, and you can directly call it with:
- mvn spotless:check : check if format is correct, fail the build if not
- mvn spotless:apply : enforce format everywhere

      <!-- formatting with scalafmt -->
      <plugin>
        <groupId>com.diffplug.spotless</groupId>
        <artifactId>spotless-maven-plugin</artifactId>
        <version>2.25.0</version>
        <configuration>
          <scala>
            <includes>
              <include>**/*.scala</include>
            </includes>
            <excludes>
              <exclude>**/ConstraintsTest.scala</exclude> <!-- make spotless scalafmt stack overflow, works on intellij: ignore -->
            </excludes>

            <scalafmt>
              <file>.scalafmt.conf</file>
            </scalafmt>

<!--            <licenseHeader>-->
<!--              <content>/* (C)$YEAR */</content>  &lt;!&ndash; or <file>${project.basedir}/license-header</file> &ndash;&gt;-->
<!--              <delimiter>package </delimiter> &lt;!&ndash;-->
<!--                note the 'package ' argument - this is a regex which identifies the top-->
<!--                of the file, be careful that all of your sources have a package declaration,-->
<!--                or pick a regex which works better for your code &ndash;&gt;-->
<!--            </licenseHeader>-->
          </scala>
        </configuration>
        <executions>
          <execution>
            <goals>
              <goal>check</goal>
            </goals>
            <phase>verify</phase> <!-- verify: default value -->
          </execution>
        </executions>
      </plugin>

To migrate, you need to first correct relative imports in rudder, because their reordering breaks build:

#!/bin/bash

# use on all scala files to correct when at root pom level:
# cd .../webapp/source
# find . -name "*.scala" -exec ~/code/refactoring-scala-format/pre-refactor.sh {} \;

FILE=$1

echo "$1 ..." 

# correct relative imports

sed -i -e "s/import Box\./import net.liftweb.common.Box./g" \
       -e "s/import JsCmds\./import net.liftweb.http.js.JsCmds./g" \
       -e "s/import JE\./import net.liftweb.http.js.JE./g" \
       -e "s/import Helpers\./import net.liftweb.util.Helpers./g" \
       -e "s/import model\.JsNodeId/import com.normation.rudder.web.model.JsNodeId/g" \
       -e "s/import http\./import net.liftweb.http./g" \
       -e "s/import js\./import net.liftweb.http.js./g" \
       -e "s/import JsExp\./import net.liftweb.http.js.JsExp./g" \
       -e "s/import common\./import net.liftweb.common./g" \
       -e "s/import LDAPConstants\./import com.normation.inventory.ldap.core.LDAPConstants./g" \
       -e "s/import S\./import net.liftweb.http.S./g" \
       -e "s/import SectionVal\./import com.normation.rudder.domain.policies.SectionVal./g" \
       -e "s/import AgentType\./import com.normation.inventory.domain.AgentType./g" \
       -e "s/import DN\./import com.unboundid.ldap.sdk.DN./g" \
       -e "s/import BuildFilter\./import com.normation.ldap.sdk.BuildFilter./g" \
       -e "s/import NoWhitespace\./import fastparse.NoWhitespace./g" \
       -e "s/import SingleLineWhitespace\./import fastparse.SingleLineWhitespace./g" \
       -e "s/import CmdbQueryParser\./import com.normation.rudder.services.queries.CmdbQueryParser./g" \
       -e "s/import JsonParser.ParseException/import net.liftweb.json.JsonParser.ParseException/g" \
       -e "s/import CfclerkXmlConstants\./import com.normation.cfclerk.xmlparsers.CfclerkXmlConstants./g" \
       -e "s/import ExecutionContext.Implicits.global/import scala.concurrent.ExecutionContext.Implicits.global/" \
${FILE}
Actions #5

Updated by François ARMAND over 1 year ago

Then, enforce formating:

mvn spotless:apply

Then, we need to correct a lot of place looking like that (comman not in the end because of comments):

    agentType:     AgentType
    // for now, the version must be an option, because we don't add it in the inventory
    // and must try to find it from packages
    ,
    version:       Option[AgentVersion],
    securityToken: SecurityToken
    // agent capabilties are lower case string used as tags giving information about what agent can do
    ,
    capabilities:  Set[AgentCapability]

We can't do that easily with sed because multiline sed is a pain, so we will do in intellij, with filter on scala file and regex replace in all files:

search:
((([ ]*//|^[ ]*/?\*).*\n)+)[ ]+,
replace with:
,$1

And finally, an other format enforce and a build to check that everything runs:

mvn spotless:apply
mvn clean test
Actions #6

Updated by Vincent MEMBRÉ over 1 year ago

  • Target version changed from 6.2.20 to old 6.2 issues to relocate
Actions #7

Updated by François ARMAND over 1 year ago

  • Target version changed from old 6.2 issues to relocate to 7.1.8

Since 6.2/7.0 are not more maintained, we need to do that only in 7.1 and up.

Actions #8

Updated by François ARMAND over 1 year ago

  • Assignee changed from François ARMAND to Vincent MEMBRÉ
Actions #9

Updated by François ARMAND over 1 year ago

  • Subtask #21995 added
Actions #10

Updated by Anonymous over 1 year ago

  • Status changed from In progress to Pending release
Actions #11

Updated by Alexis Mousset over 1 year ago

  • Subtask #21996 added
Actions #12

Updated by François ARMAND over 1 year ago

  • Subtask #21998 added
Actions #13

Updated by François ARMAND over 1 year ago

  • Subtask #21999 added
Actions #14

Updated by François ARMAND over 1 year ago

  • Subtask #22000 added
Actions #15

Updated by François ARMAND over 1 year ago

  • Subtask #22001 added
Actions #16

Updated by François ARMAND over 1 year ago

  • Related to Bug #22004: Spotless must not be exec in plugin parent added
Actions #17

Updated by François ARMAND over 1 year ago

  • Subtask #22005 added
Actions #18

Updated by François ARMAND over 1 year ago

  • Subtask #22006 added
Actions #19

Updated by François ARMAND over 1 year ago

  • Subtask #22007 added
Actions #20

Updated by François ARMAND over 1 year ago

  • Subtask #22009 added
Actions #21

Updated by Alexis Mousset over 1 year ago

  • Fix check changed from To do to Checked
Actions #22

Updated by Vincent MEMBRÉ over 1 year ago

This bug has been fixed in Rudder 7.1.8 and 7.2.2 which were released today.

Actions #23

Updated by Vincent MEMBRÉ 20 days ago

  • Status changed from Pending release to Released
Actions

Also available in: Atom PDF