sbt / contraband Goto Github PK

View Code? Open in Web Editor NEW

67.0 67.0 21.0 910 KB

http://www.scala-sbt.org/contraband/

License: Other

Scala 99.40% Java 0.60%

contraband's Introduction

sbt

sbt is a build tool for Scala, Java, and more.

For general documentation, see https://www.scala-sbt.org/.

sbt 1.x

This is the 1.x series of sbt. The source code of sbt is split across several GitHub repositories, including this one.

sbt/io hosts sbt.io module.
sbt/librarymanagement hosts sbt.librarymanagement module that wraps Ivy.
sbt/zinc hosts Zinc, an incremental compiler for Scala.
sbt/sbt, this repository hosts modules that implements the build tool.

Issues and Pull Requests

Please read CONTRIBUTING carefully before opening a GitHub Issue.

The short version: try searching or asking on StackOverflow.

license

See LICENSE.

contraband's People

Contributors

Stargazers

Watchers

contraband's Issues

src/main/datatype

The source should be src/main/datatype by default where main is for Compile configuration.
If need to I should be able to put src/test/datatype.

/cc @Duhemm

Managed source dirs aren't mapped to source directories in IDEA

Just taking a moment to start this issue.

I wouldn't be surprised if this is:

a mistake on IDEA's behalf on how they derive what the source directories are
an sbt issue
something specific to how we're using contraband in the sbt modules

self type specification is not enough for Scala 3

steps

https://github.com/sbt/sbt/blob/a53eb471b32ebb6f796830320d1a44875c5b9767/testing/src/main/contraband/testing.contra

problem

[error] -- [E058] Type Mismatch Error: /home/runner/work/sbt/sbt/testing/src/main/contraband-scala/sbt/protocol/testing/codec/TestMessageFormats.scala:9:6 
[error] 9 |trait TestMessageFormats { self: sjsonnew.BasicJsonProtocol with sbt.protocol.testing.codec.TestStringEventFormats with sbt.protocol.testing.codec.TestInitEventFormats with sbt.protocol.testing.codec.TestResultFormats with sbt.protocol.testing.codec.TestCompleteEventFormats with sbt.protocol.testing.codec.StartTestGroupEventFormats with sbt.protocol.testing.codec.EndTestGroupEventFormats with sbt.protocol.testing.codec.EndTestGroupErrorEventFormats with sbt.protocol.testing.codec.TestItemDetailFormats with sbt.protocol.testing.codec.TestItemEventFormats =>
[error]   |      ^
[error]   |missing requirement: self type sjsonnew.BasicJsonProtocol & (sbt.protocol.testing.codec.TestStringEventFormats 
[error]   |  &
[error]   | (sbt.protocol.testing.codec.TestInitEventFormats & (
[error]   |  sbt.protocol.testing.codec.TestResultFormats
[error]   | & (sbt.protocol.testing.codec.TestCompleteEventFormats & (
[error]   |  sbt.protocol.testing.codec.StartTestGroupEventFormats
[error]   | & (sbt.protocol.testing.codec.EndTestGroupEventFormats & (
[error]   |  sbt.protocol.testing.codec.EndTestGroupErrorEventFormats
[error]   | & (sbt.protocol.testing.codec.TestItemDetailFormats & 
[error]   |  sbt.protocol.testing.codec.TestItemEventFormats
[error]   |)))))))) & sbt.protocol.testing.codec.TestMessageFormats of trait TestMessageFormats does not conform to self type sbt.internal.testing.StatusFormats & sjsonnew.BasicJsonProtocol
[error]   |of required trait TestItemDetailFormats

note

The generated code works for Scala 2, but Scala 3 seems to require list of all transitive self type implied dependencies due to scala/scala3#2214

steps

## Represents a diagnostic, such as a compiler error or warning.
## Diagnostic objects are only valid in the scope of a resource.
type Diagnostic {
  ## The range at which the message applies.
  range: sbt.internal.langserver.Range!

  ## The diagnostic's severity. Can be omitted. If omitted it is up to the
  ## client to interpret diagnostics as error, warning, info or hint.
  severity: Long

  ## The diagnostic's code. Can be omitted.
  code: String

  ## A human-readable string describing the source of this
  ## diagnostic, e.g. 'typescript' or 'super lint'.
  source: String

  ## The diagnostic's message.
  message: String!
}

problem

/**
 * Represents a diagnostic, such as a compiler error or warning.
 * Diagnostic objects are only valid in the scope of a resource.
 */
final class Diagnostic private (
  /** The range at which the message applies. */
  val range: sbt.internal.langserver.Range,
  /**
   * The diagnostic's severity. Can be omitted. If omitted it is up to the
   * client to interpret diagnostics as error, warning, info or hint.
   */
  val severity: Option[Long],
  /** The diagnostic's code. Can be omitted. */
  val code: Option[String],
  /**
   * A human-readable string describing the source of this
   * diagnostic, e.g. 'typescript' or 'super lint'.
   */
  val source: Option[String],
  /** The diagnostic's message. */
  val message: String) extends Serializable

This is incorrect scaladoc, and results in "Diagnostic.scala:14:3: discarding unmoored doc comment".

expectation

/**
 * Represents a diagnostic, such as a compiler error or warning.
 * Diagnostic objects are only valid in the scope of a resource.
 * 
 * @param range The range at which the message applies.
 * @param severity The diagnostic's severity. Can be omitted. If omitted it is up to the
 *        client to interpret diagnostics as error, warning, info or hint.
 * @param code The diagnostic's code. Can be omitted.
 * @param source A human-readable string describing the source of this
 *        diagnostic, e.g. 'typescript' or 'super lint'.
 * @param message The diagnostic's message.
 */
final class Diagnostic private (
  val range: sbt.internal.langserver.Range,
  val severity: Option[Long],
  val code: Option[String],
  val source: Option[String],
  val message: String) extends Serializable

remove lightbend-cla-validator

and add scala-cla? 🤔

gh-pages could use a publish

See https://www.scala-sbt.org/contraband/

There are typo fixes and doc fixes to contraband, but the gh-pages branch is from two years ago. :-)

Generates illegal code

I've not minimised this yet. But using latest Contraband is now generating illegal code:

[error] /d/sbt-lm/librarymanagement/target/scala-2.11/src_managed/main/sbt/librarymanagement/ModuleID.scala:81: illegal start of simple expression
[error]   def apply(organization: String, name: String, revision: String): ModuleID = new ModuleID(organization, name, revision, None, false, true, false, , , , Map.empty, sbt.librarymanagement.Disabled(), None)
[error]                                                                                                                                                    ^

Support for rudimentary optics

If the plugin would be extended by an option to generate lenses, it could output additional generated code similar to this:

object MyType {
  // autogenerated code ...
  val X:Lens[MyType,Int] = Lens(_.x, _.withX)
}

This would greatly help the user of the plugin to write deeply nested property modifications in a comprehensive manner, by combining lenses. It should not matter much which of the popular Scala Optics libraries would be supported, as long as it's as least one.

Adding those lenses manually is tedious and subverts the idea of code generation.

Is it possible to define a Map within a type?

I see that there is support for StringStringMap, but trying to use a raw Map type seems to break things due to the parser handling of [.

Handle Scala reserved words

JSON should be able to handle Scala reserved words like type.

Can't define generic datatypes

Trying to convert

final case class SettingQuery[A](settingKey: SettingKey[A]) extends CommandMessage
final case class SettingQueryResponse[A](value: Option[A]) extends EventMessage

into the Contraband Schema Language, like so:

type SettingQuery[A] implements CommandMessage {
  settingKey: sbt.SettingKey[A]!
}

type SettingQueryResponse[A] implements EventMessage {
  value: A
}

results in

ParseError(Position(316,16,18), Position(316,16,18), <5 traces>)
	at org.parboiled2.Parser.done$1(Parser.scala:185)
	at org.parboiled2.Parser.phase4_collectRuleTraces$1(Parser.scala:196)
	at org.parboiled2.Parser.__run(Parser.scala:209)
	at sbt.contraband.parser.SchemaParser$.parse(SchemaParser.scala:390)
	at sbt.contraband.parser.SchemaParser$.parse(SchemaParser.scala:386)
	at sbt.contraband.Generate$$anonfun$5.apply(ContrabandPlugin.scala:134)
	at sbt.contraband.Generate$$anonfun$5.apply(ContrabandPlugin.scala:133)
	at scala.collection.TraversableLike$$anonfun$map$1.apply(TraversableLike.scala:244)
	at scala.collection.TraversableLike$$anonfun$map$1.apply(TraversableLike.scala:244)
	at scala.collection.immutable.List.foreach(List.scala:318)
	at scala.collection.TraversableLike$class.map(TraversableLike.scala:244)
	at scala.collection.AbstractTraversable.map(Traversable.scala:105)
	at sbt.contraband.Generate$.generate(ContrabandPlugin.scala:133)
	at sbt.contraband.Generate$.sbt$contraband$Generate$$gen$1(ContrabandPlugin.scala:196)
	at sbt.contraband.Generate$$anonfun$8.apply(ContrabandPlugin.scala:199)
	at sbt.contraband.Generate$$anonfun$8.apply(ContrabandPlugin.scala:199)

line: 16, column: 18 being the [ in SettingQuery[A].

Travis CI dropped

Given the poor handling of the recent security incident (https://arstechnica.com/information-technology/2021/09/travis-ci-flaw-exposed-secrets-for-thousands-of-open-source-projects/) I'm suspending Travis CI integration indefinitely.

Let's move on to GitHub Actions.

Generate incorrect equals() and hashCode() for Boolean in Java

The Java code generator doesn't produce the same code for a Boolean field if the field's type is capitalized or not. The correct code is generated for a boolean, but not for a Boolean.

It seems that isPrimitive() in JavaCodeGen.scala doesn't consider Boolean to be a primitive type because the boxed type has the same name. So genEquals() and genHashCode() produce the wrong result.

I didn't test if other primitive types have the same issue.
Here is a test case demonstrating the issue and its result:

Java code gen
- should consider Boolean as a primitive type *** FAILED ***
  Left map lines did not equal right map lines:
  --- expected/BooleanTest.java
  +++ obtained/BooleanTest.java
  @@ -25,11 +25,11 @@
   return false;
   } else {
   BooleanTest o = (BooleanTest)obj;
  -return (smallBoolean() == o.smallBoolean()) && (bigBoolean() == o.bigBoolean());
  +return (smallBoolean() == o.smallBoolean()) && bigBoolean().equals(o.bigBoolean());
   }
   }
   public int hashCode() {
  -return 37 * (37 * (17 + (new Boolean(smallBoolean())).hashCode()) + (new Boolean(bigBoolean())).hashCode());
  +return 37 * (37 * (17 + (new Boolean(smallBoolean())).hashCode()) + bigBoolean().hashCode());
   }
   public String toString() {
   return "BooleanTest("  + "smallBoolean: " + smallBoolean() + ", " + "bigBoolean: " + bigBoolean() + ")"; (JsonJavaBooleanCodeGenSpec.scala:34)

Allow eliminating and overriding companion `.apply` method

IIUC, presently, Contraband generates the companion .apply method for all the data types. It may be useful to make it optional, and override it when needed. This is useful when you want to hide the actual constructor, and provide a smart constructor.

Allow modifier to be changed from final class to sealed class

This allows companion object to extend the class.

Generating private constructors generates lots of noise

Here's a sample

[warn] /d/sbt-lm/librarymanagement/target/scala-2.11/src_managed/main/sbt/internal/librarymanagement/InlineConfiguration.scala:20: private constructor in class InlineConfiguration is never used
[warn]   private def this(validate: Boolean, ivyScala: Option[sbt.librarymanagement.IvyScala], module: sbt.librarymanagement.ModuleID, moduleInfo: sbt.librarymanagement.ModuleInfo, dependencies: Vector[sbt.librarymanagement.ModuleID]) = this(validate, ivyScala, module, moduleInfo, dependencies, Set.empty, Vector(), scala.xml.NodeSeq.Empty, Vector(), None, sbt.librarymanagement.ConflictManager.default)
[warn]               ^
[warn] /d/sbt-lm/librarymanagement/target/scala-2.11/src_managed/main/sbt/internal/librarymanagement/RetrieveConfiguration.scala:13: private constructor in class RetrieveConfiguration is never used
[warn]   private def this(retrieveDirectory: java.io.File, outputPattern: String) = this(retrieveDirectory, outputPattern, false, None)
[warn]               ^
[warn] /d/sbt-lm/librarymanagement/target/scala-2.11/src_managed/main/sbt/librarymanagement/Artifact.scala:16: private constructor in class Artifact is never used
[warn]   private def this(name: String) = this(name, Artifact.DefaultType, Artifact.DefaultExtension, None, Vector(), None, Map.empty)
[warn]               ^
[warn] /d/sbt-lm/librarymanagement/target/scala-2.11/src_managed/main/sbt/librarymanagement/Configuration.scala:15: private constructor in class Configuration is never used
[warn]   private def this(name: String) = this(name, "", true, Vector(), true)
[warn]               ^
[warn] /d/sbt-lm/librarymanagement/target/scala-2.11/src_managed/main/sbt/librarymanagement/ConflictManager.scala:13: private constructor in class ConflictManager is never used
[warn]   private def this(name: String) = this(name, "*", "*")
[warn]               ^
[warn] /d/sbt-lm/librarymanagement/target/scala-2.11/src_managed/main/sbt/librarymanagement/InclExclRule.scala:21: private constructor in class InclExclRule is never used
[warn]   private def this() = this("*", "*", "*", Vector())
[warn]               ^

Add a sjsonNewVersion settting key

Add a sjsonNewVersion setting key to the ContrabandPlugin to help adding the same version of sjson-new modules: ContrabandPlugin adds sjson-new-core and users add sjson-new-scalajson.

maven plugin

Code generation doc doesn't include interfaces

I still have no idea how the code generated looks for interfaces; some F-bounded polymorphism? The code generation docs do not include a type implementing any interfaces.

Generated code causes compiler warnings

When building sbt it generates code that causes warnings like these:

[warn] /home/neo/Forks/sbt/protocol/src/main/contraband-scala/sbt/internal/langserver/ClientCapabilities.scala:12:8: pattern var x in method equals is never used; `x@_' suppresses this warning
[warn]   case x: ClientCapabilities => true

Solution required for: sbt/sbt#3512

always use \n for line breaks

system-dependent code generation screws up diffs if the generated files are kept in vcs, like in https://github.com/sbt/librarymanagement

CSL can't handle empty definitions

interface SettingQueryResponse implements EventMessage

throws

java.lang.RuntimeException: (Some(sbt.protocol),type) not found
	at scala.sys.package$.error(package.scala:27)
	at sbt.contraband.CodeGenerator.lookupInterface(CodeGen.scala:77)
	at sbt.contraband.CodeGenerator$$anonfun$lookupInterfaces$1.apply(CodeGen.scala:64)
	at sbt.contraband.CodeGenerator$$anonfun$lookupInterfaces$1.apply(CodeGen.scala:64)
	at scala.collection.TraversableLike$$anonfun$map$1.apply(TraversableLike.scala:244)
	at scala.collection.TraversableLike$$anonfun$map$1.apply(TraversableLike.scala:244)
	at scala.collection.immutable.List.foreach(List.scala:318)
	at scala.collection.TraversableLike$class.map(TraversableLike.scala:244)
	at scala.collection.AbstractTraversable.map(Traversable.scala:105)
	at sbt.contraband.CodeGenerator.lookupInterfaces(CodeGen.scala:64)
	at sbt.contraband.ScalaCodeGen.generateInterface(ScalaCodeGen.scala:102)
	at sbt.contraband.MixedCodeGen.generateInterface(MixedCodeGen.scala:34)
	at sbt.contraband.CodeGenerator.generate(CodeGen.scala:196)
	at sbt.contraband.MixedCodeGen$$anonfun$generate$2.apply(MixedCodeGen.scala:23)
	at sbt.contraband.MixedCodeGen$$anonfun$generate$2.apply(MixedCodeGen.scala:23)
	at scala.collection.TraversableLike$$anonfun$map$1.apply(TraversableLike.scala:244)
	at scala.collection.TraversableLike$$anonfun$map$1.apply(TraversableLike.scala:244)
	at scala.collection.immutable.List.foreach(List.scala:318)
	at scala.collection.TraversableLike$class.map(TraversableLike.scala:244)
	at scala.collection.AbstractTraversable.map(Traversable.scala:105)
	at sbt.contraband.MixedCodeGen.generate(MixedCodeGen.scala:23)
	at sbt.contraband.Generate$$anonfun$6.apply(ContrabandPlugin.scala:145)
	at sbt.contraband.Generate$$anonfun$6.apply(ContrabandPlugin.scala:144)
	at scala.collection.TraversableLike$$anonfun$flatMap$1.apply(TraversableLike.scala:251)
	at scala.collection.TraversableLike$$anonfun$flatMap$1.apply(TraversableLike.scala:251)
	at scala.collection.immutable.List.foreach(List.scala:318)
	at scala.collection.TraversableLike$class.flatMap(TraversableLike.scala:251)
	at scala.collection.AbstractTraversable.flatMap(Traversable.scala:105)
	at sbt.contraband.Generate$.generate(ContrabandPlugin.scala:144)
	at sbt.contraband.Generate$.sbt$contraband$Generate$$gen$1(ContrabandPlugin.scala:196)
	at sbt.contraband.Generate$$anonfun$8.apply(ContrabandPlugin.scala:199)
	at sbt.contraband.Generate$$anonfun$8.apply(ContrabandPlugin.scala:199)
	at sbt.FileFunction$$anonfun$cached$1.apply(Tracked.scala:253)
	at sbt.FileFunction$$anonfun$cached$1.apply(Tracked.scala:253)
	at sbt.FileFunction$$anonfun$cached$2$$anonfun$apply$3$$anonfun$apply$4.apply(Tracked.scala:267)
	at sbt.FileFunction$$anonfun$cached$2$$anonfun$apply$3$$anonfun$apply$4.apply(Tracked.scala:263)
	at sbt.Difference.apply(Tracked.scala:224)
	at sbt.Difference.apply(Tracked.scala:206)
	at sbt.FileFunction$$anonfun$cached$2$$anonfun$apply$3.apply(Tracked.scala:263)
	at sbt.FileFunction$$anonfun$cached$2$$anonfun$apply$3.apply(Tracked.scala:262)
	at sbt.Difference.apply(Tracked.scala:224)
	at sbt.Difference.apply(Tracked.scala:200)
	at sbt.FileFunction$$anonfun$cached$2.apply(Tracked.scala:262)
	at sbt.FileFunction$$anonfun$cached$2.apply(Tracked.scala:260)
	at sbt.contraband.Generate$.apply(ContrabandPlugin.scala:200)
	at sbt.contraband.ContrabandPlugin$autoImport$$anonfun$baseContrabandSettings$17.apply(ContrabandPlugin.scala:63)
	at sbt.contraband.ContrabandPlugin$autoImport$$anonfun$baseContrabandSettings$17.apply(ContrabandPlugin.scala:62)

Workaround

interface SettingQueryResponse implements EventMessage {}

Doesn't clean up after itself

I'm not sure if this is common for source generators in sbt, but I've found 3 classes in the sbt/sbt codebase that were previously generated by contraband, committed to the repository, and later dereferenced.

Generate Shapeless Generic instances

The Generic (and LabelledGeneric) typeclass in shapeless implements the concepts discussed here in the Scala language. For case classes, there is a macro in Shapeless that generates Generic instances, but of course it doesn't work for contraband's pseudo case classes.
Having this would be strictly more general than the JSON stuff that contraband already supports (as JSON serializers can be generated with shapeless based on LabelledGeneric), so I think it would be a useful addition and make contraband a more complete replacement for case classes.
I'd be willing to help out with the implementation, but I'd like to get some feedback on the idea before I start writing code. What do you think?

+generateContrabands doesn't work with crossScalaVersions

When I run +generateContrabands, it will only generate the classes for the first Scala version. After that, the sbt FileFunction.cached method skips generating the code for the other Scala version.

I used the following sbt code in my local project to work around the issue:

lazy val myProject = (project in "myProject").enablePlugins(ContrabandPlugin).settings(
  inConfig(Compile)(Seq(
    scalacOptions := Seq(),
    libraryDependencies ++= (generateContrabands / contrabandCodecsDependencies).value,
    generateContrabands := {
      val generatedSourceCode = generateContrabands.value
      // clear out the gen-api cache, so that crossScalaVersions will generate the code for Scala 2.13
      (streams.value.cacheDirectory / "gen-api").delete()
      generatedSourceCode
    }
  ))
)

This would generate two separate src_managed folders in my target directory per Scala version. However, this requires regenerating the source code on every compile.

Can we use per Scala version caching rather than across all Scala versions?

Regression in constructor privacy

in sbt/librarymanagement Artifact is defined as:

    {
      "name": "Artifact",
      "namespace": "sbt.librarymanagement",
      "target": "Scala",
      "type": "record",
      "parents": "sbt.librarymanagement.ArtifactExtra",
      "fields": [
        { "name": "name",            "type": "String"                                                                                         },
        { "name": "type",            "type": "String",                               "default": "Artifact.DefaultType",      "since": "0.0.1" },
        { "name": "extension",       "type": "String",                               "default": "Artifact.DefaultExtension", "since": "0.0.1" },
        { "name": "classifier",      "type": "String?",                              "default": "",                          "since": "0.0.1" },
        { "name": "configurations",  "type": "sbt.librarymanagement.Configuration*", "default": "",                          "since": "0.0.1" },
        { "name": "url",             "type": "java.net.URL?",                        "default": "",                          "since": "0.0.1" },
        { "name": "extraAttributes", "type": "Map[String, String]",                  "default": "Map.empty",                 "since": "0.0.1" }
      ],
      "parentsCompanion": "sbt.librarymanagement.ArtifactFunctions"
    },

In contraband 0.3.0-SNAPSHOT its constructor is now private and its apply now takes String for 'classifier' instead of 'Option[String]', breaking existing code.

The workaround is to do a long with-chain to get the datatype value intended.

Allow sealed interface

Interfaces are translated to Abstract classes. Is it possible to have sealed abstract class and all it's implementation in same file?

Cannot resolve symbol ScalaKeywords

Cannot resolve symbol ScalaKeywords How to solve this

Consider allowing toString to be overloaded

Use case https://github.com/sbt/librarymanagement/blob/v0.1.0-X1/librarymanagement/src/main/scala/sbt/librarymanagement/ModuleID.scala#L12-L15

exposing copy is not safe

The default parameter values generate order dependent artificial fields.
This is not going to be safe if the user inserts field in the middle of the list.

@dwijnand might have example for this.

equal / hashCode inconsistencies around Array

@smarter reports on zinc-contrib Gitter:

Observe: https://github.com/sbt/zinc/blob/1.x/internal/compiler-interface/src/main/contraband-java/xsbti/api/Path.java
equals uses Arrays.deepEquals to compare arrays, but hashcode uses the regular array hashCode instead of Arrays.deepHashCode.

This could have severe consequence since you could end up with different hashCode while equal returning true.

The way optional properties are defaulted is wrong

Previously you could default any optional type like this:

{ "name": "defaultConfiguration", "type": "sbt.librarymanagement.Configuration?", "default": "None", "since": "0.0.1" }

Now that generates

Option(None)

as the value in the constructor overload.

Changing the default to blank string ("default": "") generates

Option()

Which is also wrong as Option.apply is defined as:

def apply[A](x: A): Option[A]

Can't override a datatype's formats

The key datatypeFormatsForType only works for types that aren't defined in the scheme.

See https://github.com/sbt/contraband/blob/v0.3.0-M1/library/src/main/scala/sbt/contraband/CodecCodeGen.scala#L320-L324

Rolling deprecation support

For a contained ecosystem, rolling deprecation feature might be useful.

type Greeting {
  value: String!
  x: Int @since("0.2.0") @deprecated("0.3.0") @remove("0.4.0")
  number: Long @since("0.3.0")
}

This could generate

object Greeting {
  // 0.0.0
  def apply(value: String): Greeting = new Greeting(value, None, None)
  // 0.2.0
  @deprecated
  def apply(value: String, x: Int): Greeting = new Greeting(value, Some(x), None)
  // 0.3.0
  def apply(value: String, number: Long): Greeting = new Greeting(value, None, Some(number))
}

Once 0.4.0 is reached, remove the field altogether:

object Greeting {
  // 0.0.0
  def apply(value: String): Greeting = new Greeting(value, None)
  // 0.2.0
  def apply(value: String, x: Int): Greeting = Macro.restligeist("0.2.0 apply no longer exists.")
  // 0.3.0
  def apply(value: String, number: Long): Greeting = new Greeting(value, Some(number))
}

Default Arguments vs Interspersed Growth (Design Decision)

While reviewing #44 @eed3si9n noted that the use of default arguments (in the companion object's apply, the constructor, and the copy method) moves the default value to the call-site, where it can't be changed in a later version of sbt when we (eg) flip the default from false to true.

Turns out (I didn't know this) that default arguments are encoded at the definition site as (for instance) copy$default$1.

This makes the order of parameters significant, whereby adding a property other than the end can easily cause binary incompatibilities. Thus @eed3si9n opened #48.

The design decision to make is whether to:

Allow the use of default arguments, but require properties to be added to the end
Allow properties to be added anywhere l, but forgo the use of default arguments and therefore the copy method and requiring abundant use of the withXYZ methods for non-required parameters, even on first construction.

To help evaluating this decision I'm going to rework my patch sbt/librarymanagement#55, with a build of sbt-datatype that doesn't use default arguments, so we can see the diff.

Generated sources aren't included in the published sources jar

As an example sbt.internal.librarymanagement.IvyConfiguration isn't included in the librarymanagement sources jar.

Generate facilities for composing a runtime lookup registry

It would be very useful if Contraband generated facilities that could then be composed together to create a registry of Manifest -> type-class instances, that can be queries at runtime.

The current workaround is to manually and exhaustively enumerate the entries.

Identifier conflict on "js" field when generating JSON codec with sjson new

In the sjson generation code, the pattern match uses the identifier of js, which can conflict with the type field if it is also named js:

import _root_.sjsonnew.{ Unbuilder, Builder, JsonFormat, deserializationError }
trait DeviceInfoFormats { self: sjsonnew.BasicJsonProtocol =>
implicit lazy val DeviceInfoFormat: JsonFormat[DeviceInfo] = new JsonFormat[DeviceInfo] {
  override def read[J](jsOpt: Option[J], unbuilder: Unbuilder[J]): DeviceInfo = {
    jsOpt match {
      case Some(js) =>
      unbuilder.beginObject(js)
      val js = unbuilder.readField[Option[Boolean]]("js")
      
      case None =>
        deserializationError("Expected JsObject but found None")
    }
  }

results in

[error] <snip>/codecs/DeviceInfoFormats.scala:13:29: type mismatch;
[error]  found   : js.type (with underlying type Option[Boolean])
[error]  required: J
[error]       unbuilder.beginObject(js)
[error]                             ^

Looks like it's at https://github.com/sbt/contraband/blob/master/library/src/main/scala/sbt/contraband/CodecCodeGen.scala#L66

The Formats for the last datatype isn't mixed into the cake

in sbt/librarymanagement the last datatype defined is UpdateReportLite, its Formats isn't mixed into LibraryManagementCodec.

Does Contraband Generated No-Arg Constructors for Java POJOs

Hi,

I read the docs and looked at the source, am I right in thinking that contraband does not generate no-arg constructor for Java classes?

If not, how much work would be required to do that. I could raise a PR if you could give me some guidelines.

The reason for requesting this is that it would make interop with certain Java libraries which require POJOs (with a no-arg constructor mandatory) easier to deal with in Scala projects.

Change the hash code implementation for empty classes

From the Zinc API:

package xsbti.api;
public abstract class Access implements java.io.Serializable {
    public Access() { super(); }

    public boolean equals(Object obj) {
        if (this == obj) {
            return true;
        } else if (!(obj instanceof Access)) {
            return false;
        } else {
            Access o = (Access)obj;
            return true;
        }
    }
    public int hashCode() { return 17; }
    public String toString() {
        return "Access("  + ")";
    }
}

Empty classes cannot have a constant as the hash code, otherwise all the empty classes are recognised to be the same, when they are not.

The implementation of hash code should change for this case.

Unimplemented JsonFormats are a potential runtime bomb

[warn] /d/sbt-lm/librarymanagement/src/main/scala/sbt/internal/librarymanagement/formats/DependencyResolverFormat.scala:7: dead code following this construct
[warn]   implicit lazy val DependencyResolverFormat: JsonFormat[DependencyResolver] = ???
[warn]                                                                                ^
[warn] /d/sbt-lm/librarymanagement/src/main/scala/sbt/internal/librarymanagement/formats/GlobalLockFormat.scala:7: dead code following this construct
[warn]   implicit lazy val GlobalLockFormat: JsonFormat[GlobalLock] = ???
[warn]                                                                ^
[warn] /d/sbt-lm/librarymanagement/src/main/scala/sbt/internal/librarymanagement/formats/LoggerFormat.scala:7: dead code following this construct
[warn]   implicit lazy val LoggerFormat: JsonFormat[Logger] = ???
[warn]                                                        ^

These types are transitively required by datatypes we define with contraband. At any point there could be code that compiles that then explodes at runtime when it hits these code paths.

toString for `Array[T]` in Java code

...is currently not implemented, so the default Array toString is used:

classpath: [Ljava.io.File;@7f7b4f2c

Can't define singleton types

There's no support in Contraband to define a singleton type. For example the singleton type Disabled of the CrossVersion type family.

Some generated code does not compile with -Xfatal-warnings

I generated the following contraband file in sbt:

package sbt.internal

type MiniUpdateConfiguration {
  name: String!
  classpath: [java.io.File]
}
type MiniUpdateReport {
  configs: [MiniUpdateConfiguration]!
}

When I compile after running generateContrabands, I get the following error:

[error] ${BASE}/main/src/main/contraband-scala/sbt/internal/MiniUpdateReport.scala:22:26: private default argument in class MiniUpdateReport is never used                              
[error]   private[this] def copy(configs: Vector[MiniUpdateConfiguration] = configs): MiniUpdateReport = {

If I add another field to MiniUpdateReport, the warnings go away. It looks like maybe the generated code shouldn't use default arguments? I suppose that would require inlining the defaults everywhere in the various with* methods.

I can work around the issue but figured I should report it.

The way seq-like properties (Foo*) are defaulted makes it impossible to migrate

The previous working definition

{ "name": "scalaArtifacts", "type": "String*", "default": "sbt.librarymanagement.ScalaArtifacts.Artifacts", "since": "0.0.1" }

no longer works because sbt.librarymanagement.ScalaArtifacts.Artifacts is wrapped in a Vector, when it shouldn't be.

Scala code should be formatted with Scalafmt

Following the standards decided in sbt/sbt#3125.

Unused import warning

sbt/sbt is built with unused import warning, which gets triggered by contraband generated datatypes.

[warn] /xxx/sbt/protocol/src/main/contraband-scala/sbt/protocol/codec/ChannelAcceptedEventFormats.scala:7: Unused import
[warn] import _root_.sjsonnew.{ deserializationError, serializationError, Builder, JsonFormat, Unbuilder }
[warn]                                                ^
[warn] /xxx/sbt/protocol/src/main/contraband-scala/sbt/protocol/codec/CommandMessageFormats.scala:7: Unused import
[warn] import _root_.sjsonnew.{ deserializationError, serializationError, Builder, JsonFormat, Unbuilder }
[warn]                          ^
[warn] /xxx/sbt/protocol/src/main/contraband-scala/sbt/protocol/codec/CommandMessageFormats.scala:7: Unused import
[warn] import _root_.sjsonnew.{ deserializationError, serializationError, Builder, JsonFormat, Unbuilder }
[warn]                                                ^
[warn] /xxx/sbt/protocol/src/main/contraband-scala/sbt/protocol/codec/CommandMessageFormats.scala:7: Unused import
[warn] import _root_.sjsonnew.{ deserializationError, serializationError, Builder, JsonFormat, Unbuilder }
[warn]                                                                    ^
[warn] /xxx/sbt/protocol/src/main/contraband-scala/sbt/protocol/codec/CommandMessageFormats.scala:7: Unused import
[warn] import _root_.sjsonnew.{ deserializationError, serializationError, Builder, JsonFormat, Unbuilder }
[warn]                                                                                         ^
[warn] /xxx/sbt/protocol/src/main/contraband-scala/sbt/protocol/codec/EventMessageFormats.scala:7: Unused import
[warn] import _root_.sjsonnew.{ deserializationError, serializationError, Builder, JsonFormat, Unbuilder }
[warn]                          ^
[warn] /xxx/sbt/protocol/src/main/contraband-scala/sbt/protocol/codec/EventMessageFormats.scala:7: Unused import
[warn] import _root_.sjsonnew.{ deserializationError, serializationError, Builder, JsonFormat, Unbuilder }
[warn]                                                ^
[warn] /xxx/sbt/protocol/src/main/contraband-scala/sbt/protocol/codec/EventMessageFormats.scala:7: Unused import
[warn] import _root_.sjsonnew.{ deserializationError, serializationError, Builder, JsonFormat, Unbuilder }
[warn]                                                                    ^
[warn] /xxx/sbt/protocol/src/main/contraband-scala/sbt/protocol/codec/EventMessageFormats.scala:7: Unused import
[warn] import _root_.sjsonnew.{ deserializationError, serializationError, Builder, JsonFormat, Unbuilder }
[warn]                                                                                         ^
[warn] /xxx/sbt/protocol/src/main/contraband-scala/sbt/protocol/codec/ExecCommandFormats.scala:7: Unused import
[warn] import _root_.sjsonnew.{ deserializationError, serializationError, Builder, JsonFormat, Unbuilder }
[warn]                                                ^
[warn] /xxx/sbt/protocol/src/main/contraband-scala/sbt/protocol/codec/ExecStatusEventFormats.scala:7: Unused import
[warn] import _root_.sjsonnew.{ deserializationError, serializationError, Builder, JsonFormat, Unbuilder }
[warn]                                                ^
[warn] /xxx/sbt/protocol/src/main/contraband-scala/sbt/protocol/codec/ExecutionEventFormats.scala:7: Unused import
[warn] import _root_.sjsonnew.{ deserializationError, serializationError, Builder, JsonFormat, Unbuilder }
[warn]                                                ^
[warn] /xxx/sbt/protocol/src/main/contraband-scala/sbt/protocol/codec/LogEventFormats.scala:7: Unused import
[warn] import _root_.sjsonnew.{ deserializationError, serializationError, Builder, JsonFormat, Unbuilder }

use "boxed type".hashCode(value) instead of "boxed type".valueOf(value).hashCode()

I think we can/should use these methods if contraband does not support JDK 7.

https://docs.oracle.com/javase/8/docs/api/java/lang/Integer.html#hashCode-int-

hashCode
public static int hashCode(int value)
Returns a hash code for a int value; compatible with Integer.hashCode().
Parameters:
value - the value to hash
Returns:
a hash code value for a int value.
Since:
1.8

plugin name

The plugin should be named sbt-datatype.

/cc @Duhemm

sbt / contraband Goto Github PK

contraband's Introduction

sbt

sbt 1.x

Other links

Issues and Pull Requests

license

contraband's People

Contributors

Stargazers

Watchers

Forkers

contraband's Issues

steps

problem

note

steps

problem

expectation

Workaround

Recommend Projects

Recommend Topics

Recommend Org