Architecture #21741
closedAuto-format scala code
Added by François ARMAND about 2 years ago. Updated 8 months ago.
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
Updated by François ARMAND about 2 years ago
- Status changed from New to In progress
- Assignee set to François ARMAND
Updated by François ARMAND about 2 years 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
Updated by François ARMAND about 2 years ago
- Status changed from Pending technical review to In progress
- Assignee changed from Vincent MEMBRÉ to François ARMAND
Updated by François ARMAND about 2 years 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> <!– or <file>${project.basedir}/license-header</file> –>--> <!-- <delimiter>package </delimiter> <!–--> <!-- 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 –>--> <!-- </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}
Updated by François ARMAND about 2 years 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
Updated by Vincent MEMBRÉ about 2 years ago
- Target version changed from 6.2.20 to old 6.2 issues to relocate
Updated by François ARMAND about 2 years 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.
Updated by François ARMAND about 2 years ago
- Assignee changed from François ARMAND to Vincent MEMBRÉ
Updated by Anonymous about 2 years ago
- Status changed from In progress to Pending release
Applied in changeset rudder|c9800ee5d915c9efa6410027c37ab86cbe6a44d6.
Updated by François ARMAND about 2 years ago
- Related to Bug #22004: Spotless must not be exec in plugin parent added
Updated by Alexis Mousset almost 2 years ago
- Fix check changed from To do to Checked
Updated by Vincent MEMBRÉ almost 2 years ago
This bug has been fixed in Rudder 7.1.8 and 7.2.2 which were released today.
Updated by Vincent MEMBRÉ 8 months ago
- Status changed from Pending release to Released