diff options
author | Christopher Vogt <oss.nsp@cvogt.org> | 2017-03-11 17:01:30 -0500 |
---|---|---|
committer | Christopher Vogt <oss.nsp@cvogt.org> | 2017-03-11 18:30:24 -0500 |
commit | 3f68499ec4768e2ae1bfe2e390ba66a90b190fc0 (patch) | |
tree | d9c85a9bef52d22d1db697bd9276bedca581c547 | |
parent | 163bbc8f1d78750196b30a46a888e7e65ace5454 (diff) | |
download | cbt-3f68499ec4768e2ae1bfe2e390ba66a90b190fc0.tar.gz cbt-3f68499ec4768e2ae1bfe2e390ba66a90b190fc0.tar.bz2 cbt-3f68499ec4768e2ae1bfe2e390ba66a90b190fc0.zip |
Document CBT plugin style guide and adjust Scalafmt plugin accordingly
-rw-r--r-- | doc/plugin-author-guide.md | 105 | ||||
-rw-r--r-- | libraries/proguard/build/build.scala | 2 | ||||
-rw-r--r-- | plugins/scalafmt/Scalafmt.scala | 21 |
3 files changed, 114 insertions, 14 deletions
diff --git a/doc/plugin-author-guide.md b/doc/plugin-author-guide.md new file mode 100644 index 0000000..db0eede --- /dev/null +++ b/doc/plugin-author-guide.md @@ -0,0 +1,105 @@ +## How to write an idiomatic CBT plugin? + +Write a small library that could be fully used outside of CBT's +build classes and handles all the use cases you need. For example + +``` +object MyLibrary{ + def doSomething( ... ) = // do something here +} +``` + +Publish it as a library and you might be done right here. + +If your library requires configuration information commonly found +in your build, like the sourceFiles, groupId, scalaVersion or else, +consider offering a mixin trait, that pre-configures your library +for user convenience. (Consider publishing them separately if that +allows people to use your library outside of CBT with fewer +dependencies.) Here is an example of a library with an +accompanying mixin trait configuring the library for CBT. + +``` +package my.library +object MyLibrary{ + case class DoSomething( scalaVersion: String, ... ){ + case class config( targetFile: File, affectBehavior: Boolean = true, ... ){ + def apply = // really do something here + } + } +} + +package my.plugin +trait MyLibrary extends BaseBuild{ + def doSomething = MyLibrary.DoSomething(scalaVersion, ...).config( scalaTarget / "my.file" ) +} +``` + +* Note: Do not override any common method like `compile` or `test` in a public plugin. * +* Instead document recommendations where users should hook in your custom methods. * +* This will help users understand their own builds. * + +See how we only define a single `def doSomething` in the MyLibrary +trait? We did not define things like `def doSomethingTargetFile`. +Instead we have a case class defined in the library which a user +can .copy as needed to customize configuration. A user build could +look like this: + +``` +class Build(val context: Context) extends MyLibrary{ + override def doSomething = super.doSomething.copy( + affectBehavior = false + ) +} +``` + +As you can see a user can use .copy to override default behavior +of your library. + +This nesting allows us to keep the global namespace small, which +helps us lower the risk of global name clashes between different +libraries. It also makes it clearer that `affectBehavior` is +something specific to `MyLibary` which should help making builds +easier to understand. Further this nesting means we don't need to +encode namespaces in the names themselves, but use Scala's language +features for that, which allows us to keep names nice and concise. + +You might wonder why there is a case class `DoSomething` rather +than `scalaVersion` just being another parameter in case class +`config`. Nesting case classes like this is a pattern that given +the way we use them allows us to make it slightly harder to +modify some parameters (the ones on the outer case class) than +others (the ones on the inner case class). Why? Some are likely +to need user customization, others are likely to break stuff if +they are touched. Example: The scalaVersion is probably something +you want to configure once consistently across your entire build. +Otherwise you might end up accidentally packaging scala-2.11 +compiled class files as a jar with a `_2.10` artifact id. +Changing which targetFile `doSomething` writes to however is +something you should be able to safely change. Since we defined +`doSomething` as +`def doSomething = MyLibrary.DoSomething(...).config( ... )` +overriding behavior in user code with .copy only affects +the inner case class because super.doSomething is an instance +of that: +``` + override def doSomething = super.doSomething.copy( + affectBehavior = false + ) +``` +Overriding things in class `DoSomething` is possible by creating +an entire new instance of the outer one, but slightly harder +preventing users from accidentally doing the wrong thing. + +Obviously this decisions what's dangerous to override and what +is not can be a judgment call and not 100% clear. + +A few more conventions for more uniform plugin designs: +If you only have one outer case class in yur plugin `object`, +call the case class `apply` instead of `DoSomething`. If you +need multiple (because you basically have several commands, +each with their own private configuration), give it a name +representing the operation, e.g. `compile` or `doc`. If there +is only one inner case class inside of anothe case class, +call it `config`, give it a name representing the operation, +e.g. `compile` or `doc`. diff --git a/libraries/proguard/build/build.scala b/libraries/proguard/build/build.scala index c781ce2..3ca38b5 100644 --- a/libraries/proguard/build/build.scala +++ b/libraries/proguard/build/build.scala @@ -15,7 +15,7 @@ class Build(val context: Context) extends Scalafmt{ } override def scalafmt = super.scalafmt.copy( - config = super.scalafmt.lib.cbtRecommendedConfig, + config = Scalafmt.cbtRecommendedConfig, whiteSpaceInParenthesis = true ) diff --git a/plugins/scalafmt/Scalafmt.scala b/plugins/scalafmt/Scalafmt.scala index 9d42cbd..5535964 100644 --- a/plugins/scalafmt/Scalafmt.scala +++ b/plugins/scalafmt/Scalafmt.scala @@ -12,16 +12,15 @@ import java.nio.file._ trait Scalafmt extends BaseBuild { /** Reformat scala source code according to `scalafmtConfig` rules */ def scalafmt = { - val scalafmtLib = new ScalafmtLib(lib) - scalafmtLib.format( sourceFiles ).config( - scalafmtLib.loadConfig( + Scalafmt.apply( lib, sourceFiles.filter(_.string endsWith ".scala") ).config( + Scalafmt.loadConfig( projectDirectory.toPath ) getOrElse ScalafmtConfig.default ) } } -class ScalafmtLib(lib: Lib){ scalafmtLib => +object Scalafmt{ def userHome = Option( System.getProperty("user.home") ).map(Paths.get(_)) /** Tries to load config from .scalafmt.conf in given directory or fallback directory */ @@ -34,22 +33,18 @@ class ScalafmtLib(lib: Lib){ scalafmtLib => .flatMap ( file => StyleCache.getStyleForFile(file.toString) ) } - case class format(files: Seq[File]){ - /** - * @param whiteSpaceInParenthesis more of a hack to make up for missing support in Scalafmt. Does not respect alignment and maxColumn. - */ + case class apply( lib: Lib, files: Seq[File] ){ + /** @param whiteSpaceInParenthesis more of a hack to make up for missing support in Scalafmt. Does not respect alignment and maxColumn. */ case class config( - config: ScalafmtConfig, - whiteSpaceInParenthesis: Boolean = false + config: ScalafmtConfig, whiteSpaceInParenthesis: Boolean = false ) extends (() => Seq[File]){ - def lib = scalafmtLib def apply = { - val (successes, errors) = scalafmtLib.lib.transformFilesOrError( + val (successes, errors) = lib.transformFilesOrError( files, org.scalafmt.Scalafmt.format(_, config) match { case Formatted.Success(formatted) => Right( if( whiteSpaceInParenthesis ){ - scalafmtLib.whiteSpaceInParenthesis(formatted) + Scalafmt.whiteSpaceInParenthesis(formatted) } else formatted ) case Formatted.Failure( e ) => Left( e ) |