Skip to content

Update scalafmt-core to 3.8.6 #1413

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .scalafmt.conf
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ rewrite.rules = [
rewrite.trailingCommas.style = never
runner.dialect = scala213
spaces.inImportCurlyBraces = true
version = 3.8.3
version = 3.8.6
fileOverride {
"glob:**/src/main/scala-3/**" {
runner.dialect = scala3
Expand Down
10 changes: 7 additions & 3 deletions daffodil-cli/src/main/scala/org/apache/daffodil/cli/Main.scala
Original file line number Diff line number Diff line change
Expand Up @@ -140,7 +140,10 @@ class CLIConf(arguments: Array[String], stdout: PrintStream, stderr: PrintStream
s match {
case Nil => Right(None) // flag was not present
case (_, Nil) :: Nil => Right(Some(None)) // flag was present but had no parameter
case (_, v :: Nil) :: Nil => { // flag was present with a parameter, convert the parameter
case (
_,
v :: Nil
) :: Nil => { // flag was present with a parameter, convert the parameter
Comment on lines -143 to +146
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Curious why some lines get expanded like this and others get compressed onto a single line.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can't really say. I imagine this has to do with long lines and splitting function parameters per line.

We really shouldn't disable scalafmt, so if we don't like these changes, we either need to find and modify a config option that changes it (not sure what that would be or if it would affect other places), open a bug with scalafmt if we think it's wrong, or rewrite it in some way that doesn't require this formatting. Of those, the last is probably the best option, but I'm not sure how much it's wroth it.

I generally let scalafmt do its thing for existing code and just focus on ensuring new code is formatted reasonably.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I took a little look into changing some settings and I think adding

newlines.avoidForSimpleOverflow = [slc]

helps a bit as that will ignore trailing single line comments when determining if we should create a new line. Tried adjusting some settings for XML literals too, but that just made things worse. You can see what the changes would look like here:

main...jadams-tresys:incubator-daffodil:scalafmt

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The avoidForSimpleOverflow=[slc] is definitely a reasonable improvement. Loosening line length restrictions in favor a readability is usually a win. That said, in most of the changes I would think just moving the comment to its own line is probably more readable, but I'm not sure it's worth the effort to fix.

try {
Right(Some(Some(conv(v))))
} catch {
Expand Down Expand Up @@ -1354,8 +1357,9 @@ class Main(
else ""
val dataHex =
if (destArrayFilled)
s"\nLeft over data (Hex) starting at byte ${curBytePosition1b} is: (0x${destArray.map { a =>
f"$a%02x"
s"\nLeft over data (Hex) starting at byte ${curBytePosition1b} is: (0x${destArray.map {
a =>
f"$a%02x"
}.mkString}...)"
else ""
val remainingBits =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -631,8 +631,7 @@ trait ElementBase
// if they have binary representation we must worry about packed digits, which require 4-bit alignment.
//
primType match {
case PrimType.Float | PrimType.Double | PrimType.Boolean |
PrimType.HexBinary => /* Non textual data, no need to compare alignment to encoding's expected alignment */
case PrimType.Float | PrimType.Double | PrimType.Boolean | PrimType.HexBinary => /* Non textual data, no need to compare alignment to encoding's expected alignment */
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This one got compressed onto a single line.

case _ =>
binaryNumberRep match {
case BinaryNumberRep.Packed | BinaryNumberRep.Bcd |
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -116,6 +116,7 @@ trait RequiredOptionalMixin { self: ElementBase =>
case (1, 1) => false
case (_, n) if n > 1 => true
case (_, UNBOUNDED) => true

/**
* This next case is for occursCountKinds parsed and stopValue.
* These only use min/maxOccurs for validation, so anything
Expand All @@ -127,6 +128,7 @@ trait RequiredOptionalMixin { self: ElementBase =>
occursCountKind == OccursCountKind.StopValue ||
occursCountKind == OccursCountKind.Expression) =>
true

/**
* Special case for minoccurs 0 and maxOccurs 0 when OCK is 'implicit' in that
* case we treat as an array that cannot have any occurrences.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -199,14 +199,16 @@ trait SchemaComponent
case ct: ComplexTypeBase => "ct"
case st: SimpleTypeDefBase => "st=" + st.namedQName.toQNameString
case cgr: ChoiceGroupRef =>
"cgr" + (if (cgr.isHidden) "h" else "") + (if (cgr.position > 1) cgr.position
else
"") + "=" + cgr.groupDef.namedQName.toQNameString
"cgr" + (if (cgr.isHidden) "h"
else "") + (if (cgr.position > 1) cgr.position
else
"") + "=" + cgr.groupDef.namedQName.toQNameString
case cgd: GlobalChoiceGroupDef => "cgd=" + cgd.namedQName.toQNameString
case sgr: SequenceGroupRef =>
"sgr" + (if (sgr.isHidden) "h" else "") + (if (sgr.position > 1) sgr.position
else
"") + "=" + sgr.groupDef.namedQName.toQNameString
"sgr" + (if (sgr.isHidden) "h"
else "") + (if (sgr.position > 1) sgr.position
else
"") + "=" + sgr.groupDef.namedQName.toQNameString
case sgd: GlobalSequenceGroupDef => "sgd=" + sgd.namedQName.toQNameString
case cg: Choice => "c" + (if (cg.position > 1) cg.position else "")
case sg: LocalSequence => "s" + (if (sg.position > 1) sg.position else "")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1304,7 +1304,10 @@ trait ElementBaseGrammarMixin
}

private lazy val nilOrValue =
prod("nilOrValue", isNillable) { // TODO: make it exclude emptyness once emptyness is implemented
prod(
"nilOrValue",
isNillable
) { // TODO: make it exclude emptyness once emptyness is implemented
SimpleNilOrValue(this, nilLit || parsedNil, parsedValue)
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -338,19 +338,24 @@ class TestDFDLExpressionTree extends Parsers {
@Test def test_numbers() = {

testExpr(dummySchema, "{ 5.0E2 }") {
case WholeExpression(_, LiteralExpression(500.0), _, _, _, _) => /* ok */ ;
case WholeExpression(_, LiteralExpression(500.0), _, _, _, _) => /* ok */
;
Comment on lines -341 to +342
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not really a fan of these changes, is it worth configuring this (if possible)?

}
testExpr(dummySchema, "{ 5E2 }") {
case WholeExpression(_, LiteralExpression(500.0), _, _, _, _) => /* ok */ ;
case WholeExpression(_, LiteralExpression(500.0), _, _, _, _) => /* ok */
;
}
testExpr(dummySchema, "{ .2E2 }") {
case WholeExpression(_, LiteralExpression(20.0), _, _, _, _) => /* ok */ ;
case WholeExpression(_, LiteralExpression(20.0), _, _, _, _) => /* ok */
;
}
testExpr(dummySchema, "{ .2E-3 }") {
case WholeExpression(_, LiteralExpression(0.0002), _, _, _, _) => /* ok */ ;
case WholeExpression(_, LiteralExpression(0.0002), _, _, _, _) => /* ok */
;
}
testExpr(dummySchema, "{ .2E+3 }") {
case WholeExpression(_, LiteralExpression(200.0), _, _, _, _) => /* ok */ ;
case WholeExpression(_, LiteralExpression(200.0), _, _, _, _) => /* ok */
;
}

// testExpr(dummySchema, "0.") { actual =>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,8 @@ object UnspecifiedNamespace extends NS(null) {
override def explainForMsg = "with unspecified namespace"
}

sealed class NS protected (uriArg: URI) extends Serializable { // protected constructor. Must use factory.
sealed class NS protected (uriArg: URI)
extends Serializable { // protected constructor. Must use factory.
override def toString = uri.toString
def uri = uriArg
def optURI = One(uriArg)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -142,6 +142,7 @@ final class RuntimeExpressionDPath[T <: AnyRef](
state.setFailed(e)
null
}

/**
* Almost anything that can go wrong in an expression maps to a SDE.
* Except when we're evaluating a discriminator.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -468,6 +468,7 @@ class VariableMap private (

case _ => {
vrd.direction match {

/**
* Due to potential race conditions regarding the setting of
* variables via setVariable and default values in combination with
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -249,7 +249,9 @@ class TestUnicodeErrorTolerance {
)
val input = new ByteArrayInputStream(inBuf);
val e =
intercept[MalformedInputException] { // fails to convert because there is no possible surrogate-pair rep for this.
intercept[
MalformedInputException
] { // fails to convert because there is no possible surrogate-pair rep for this.
Comment on lines -252 to +254
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is just harder to read in my opinion

Converter.parse(input, decoder)
}
assertEquals(1, e.getInputLength())
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,6 @@ import java.nio.LongBuffer
import java.nio.charset.CoderResult
import java.nio.charset.StandardCharsets
import scala.collection.mutable
import scala.language.postfixOps
import scala.util.Try
import scala.util.Using
import scala.xml.Elem
Expand Down Expand Up @@ -2407,7 +2406,7 @@ case class Document(d: NodeSeq, parent: TestCase) {
final lazy val nBits: Long =
documentParts.map {
_.nBits
} sum
}.sum

final lazy val documentBytes = bits2Bytes(documentBits)

Expand Down Expand Up @@ -2644,7 +2643,7 @@ sealed abstract class DataDocumentPart(part: Node, parent: Document)

lazy val lengthInBits = dataBits.map {
_.length
} sum
}.sum
override lazy val nBits: Long = lengthInBits

lazy val contentAsBits = dataBits
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -342,7 +342,9 @@ class TestTDMLRunner {
fileWriter.write(testSchema.toString())
}

val testSuite = <testSuite xmlns={tdml} suiteName="theSuiteName">
val testSuite = <testSuite xmlns={
tdml
} suiteName="theSuiteName">
Comment on lines -345 to +347
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This also seems harder to read.

<parserTestCase name="testRunTDMLFileReferencingModelFile" root="data" model={
tmpSchemaFile.toURI.toString
}>
Expand Down
6 changes: 3 additions & 3 deletions project/OsgiCheck.scala
Original file line number Diff line number Diff line change
Expand Up @@ -73,10 +73,10 @@ object OsgiCheckPlugin extends AutoPlugin {
// flatten all of our tuples so we have a single list of package name ->
// project owner tuples. At this point, there might be multiple owners
// for the same package name in this list
val packageOwnerTuples = subprojectPackagesAndOwner.flatMap {
case (ownedPackages, packageOwner) =>
val packageOwnerTuples =
subprojectPackagesAndOwner.flatMap { case (ownedPackages, packageOwner) =>
ownedPackages.map { _ -> packageOwner }
}
}

// create a map, grouping with a key of package name and value of all of the
// owners that claim to own it. If only one project owns a package, the value
Expand Down
3 changes: 2 additions & 1 deletion scripts/osgi-refactor/gen-symbol-table.scala
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,8 @@ object GenSymbolTable extends App {
val files = dir.listFiles()
for (file <- files) {
val name = file.getName
if (name.startsWith("Test") || name == "test") {} else if (file.isDirectory)
if (name.startsWith("Test") || name == "test") {}
else if (file.isDirectory)
symbols ++= processDir(libraryName, file)
else if (name.endsWith(".scala") || name.endsWith(".java")) {
processFile(libraryName, file) match {
Expand Down
Loading