Skip to content

Update scalastyle, switch to Scala-based parsing, adopt new Engines requirements #8

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

Merged
merged 17 commits into from
Dec 11, 2017
Merged
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
6 changes: 6 additions & 0 deletions .codeclimate.yml
Original file line number Diff line number Diff line change
@@ -1,3 +1,9 @@
---
engines:
scalastyle:
enabled: true
config:
config: src/main/resources/scalastyle_project.xml
exclude_paths:
- "**/src/test/"
- "**/target/"
3 changes: 3 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
target
.idea

Copy link

Choose a reason for hiding this comment

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

This repo might benefit from a .dockerignore file as well.

I don't think the build.sbt or codeclimate-scalastyle-assembly-0.1.0.jar files need to be within the final image.

Choose a reason for hiding this comment

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

codeclimate-scalastyle-assembly-0.1.0.jar is required - it has all the logic for the engine. It is copied as "/usr/src/app/engine-core.jar".
Only files in "src/main/resources/docker" and this jar file are included in the docker image.

Copy link

Choose a reason for hiding this comment

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

I think once it's COPY'd in, as /usr/src/app/engine-core.jar you can leave the original jar out of the final image.

So if a .dockerignore file included codeclimate-scalastyle-assembly-0.1.0.jar, that would remove the duplicate file, right?

Choose a reason for hiding this comment

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

Dockerfile only copies these files:

COPY codeclimate-scalastyle-assembly-0.1.0.jar /usr/src/app/engine-core.jar
COPY src/main/resources/docker /usr/src/app
COPY src/main/resources/docker/engine.json /

Thus codeclimate-scalastyle-assembly-0.1.0.jar is included only once in the docker image.

Adding it to .dockerignore will break the build process because Docker wouldn't be able to find this jar.

Copy link

Choose a reason for hiding this comment

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

Ahh, I see. You're right. Thought there was a broader COPY further down, but after looking again, there isn't. LGTM.

23 changes: 13 additions & 10 deletions Dockerfile
Original file line number Diff line number Diff line change
@@ -1,16 +1,19 @@
FROM java
FROM openjdk:alpine

RUN apt-get update
RUN apt-get install -y ruby ruby-nokogiri
LABEL maintainer "Ivan Luzyanin <[email protected]>"
LABEL maintainer "Jeff Sippel <[email protected]>"

RUN adduser --uid 9000 --disabled-password --quiet --gecos "" app
USER app
RUN apk update && apk upgrade

WORKDIR /home/app
RUN addgroup -g 9000 -S code && \
adduser -S -G code app
USER app

COPY scalastyle_config.xml /home/app/
COPY scalastyle_2.11-0.6.0-batch.jar /home/app/
COPY codeclimate-scalastyle-assembly-0.1.0.jar /usr/src/app/engine-core.jar
COPY src/main/resources/docker /usr/src/app
COPY src/main/resources/docker/engine.json /

COPY . /home/app
WORKDIR /code
VOLUME /code

CMD ["/home/app/bin/scalastyle"]
CMD ["/usr/src/app/bin/scalastyle"]
9 changes: 9 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,15 @@

`scalastyle` is a configurable style linter for Scala code.

### Building
1. Install [sbt](http://www.scala-sbt.org/)
2. Run `sbt docker`


### Building release docker image
1. Run `sbt assembly && cp target/scala-2.12/codeclimate-scalastyle-assembly-<version>.jar ./`.
This will create assembled jar with all dependencies.
2. Run `docker build -t codeclimate/codeclimate-scalastyle .`
Copy link

Choose a reason for hiding this comment

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

This is fine for now, but it would be great to have this step integrated within the docker image build process, maybe via a multi-stage Dockerfile: https://docs.docker.com/engine/userguide/eng-image/multistage-build

Choose a reason for hiding this comment

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

As I understand SBT build tool is not supported thus this is the only workaround I could think of.

Choose a reason for hiding this comment

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

There are quite a few SBT images out there. Docker definitely can run it. Or have I misunderstood your words?

Choose a reason for hiding this comment

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

Oh, I get what you are saying. Basically use a docker image with sbt to build and assembly package and copy it into the final image. I'll do that.


### Installation

Expand Down
14 changes: 0 additions & 14 deletions bin/scalastyle

This file was deleted.

78 changes: 78 additions & 0 deletions build.sbt
Original file line number Diff line number Diff line change
@@ -0,0 +1,78 @@
name := "codeclimate-scalastyle"
organization in ThisBuild := "codeclimate"
version in ThisBuild := "0.1.0"
scalaVersion in ThisBuild := "2.12.4"

concurrentRestrictions in Global += Tags.limit(Tags.Test, 1)
parallelExecution in Global := false

lazy val `engine-core` = project settings (
libraryDependencies ++= Seq(
"org.scalastyle" %% "scalastyle" % "1.0.0",
"io.circe" %% "circe-parser" % "0.8.0",
"io.circe" %% "circe-generic" % "0.8.0",
"com.github.scopt" %% "scopt" % "3.7.0",
"org.scalactic" %% "scalactic" % "3.0.4",
"org.scalatest" %% "scalatest" % "3.0.4" % "test"
)
)

lazy val `codeclimate-scalastyle` = project in file(".") dependsOn `engine-core`

resolvers in ThisBuild ++= Seq(
Resolver.sonatypeRepo("snapshots"),
Resolver.sonatypeRepo("releases")
)

enablePlugins(sbtdocker.DockerPlugin)

imageNames in docker := Seq(
// Sets the latest tag
ImageName(s"codeclimate/${name.value}:latest"),

// Sets a name with a tag that contains the project version
ImageName(
namespace = Some("codeclimate"),
repository = name.value,
tag = Some(version.value)
)
)

dockerfile in docker := {
val dockerFiles = {
val resources = (unmanagedResources in Runtime).value
val dockerFilesDir = resources.find(_.getPath.endsWith("/docker")).get
resources.filter(_.getPath.contains("/docker/")).map { r =>
(dockerFilesDir.toURI.relativize(r.toURI).getPath, r)
}.toMap
}

new Dockerfile {
from("openjdk:alpine")

// add all dependencies to docker image instead of assembly (layers the dependencies instead of huge assembly)
val dependencies = {
((dependencyClasspath in Runtime) in `engine-core`).value
}.map(_.data).toSet + ((packageBin in Compile) in `engine-core`).value

maintainer("Jeff Sippel", "[email protected]")
maintainer("Ivan Luzyanin", "[email protected]")

add(dependencies.toSeq, "/usr/src/app/dependencies/")
add(((packageBin in Compile) in `engine-core`).value, "/usr/src/app/engine-core.jar")
add(dockerFiles("scalastyle_config.xml"), "/usr/src/app/")
add(dockerFiles("engine.json"), "/")
add(dockerFiles("bin/scalastyle"), "/usr/src/app/bin/")

runRaw("apk update && apk upgrade")

runRaw("addgroup -g 9000 -S code && adduser -S -G code app")

user("app")

workDir("/code")
volume("/code")

cmd("/usr/src/app/bin/scalastyle")
}
}
Binary file added codeclimate-scalastyle-assembly-0.1.0.jar
Binary file not shown.
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
package org.codeclimate.scalastyle

import java.io.File
import java.nio.charset.Charset
import java.nio.file.Files

import io.circe.parser.parse
import org.scalastyle._

import scala.collection.JavaConverters._
import scala.util.Try

object CodeClimateEngine extends App {
case class ProgramArgs(config_file_path: String, workspace_path: String)

val argsParser = new scopt.OptionParser[ProgramArgs]("scopt") {
opt[String]("config_file_path").action { case (path, conf) =>
conf.copy(config_file_path = path)
}

opt[String]("workspace_path").action { case (path, conf) =>
conf.copy(workspace_path = path)
}
}

val defaultStyleConfigurationPath = "/usr/src/app/scalastyle_config.xml"

argsParser.parse(args, ProgramArgs("/config.json", "/code")) match {
case Some(programArgs) =>
val configFile = new File(programArgs.config_file_path)
val configJson = Try {
Files.readAllLines(configFile.toPath, Charset.defaultCharset()).asScala.toSeq.mkString("\n")
}.toEither

val providedConfig = configJson.right.flatMap(parse).right
.map(config => config.hcursor)

val includePaths = providedConfig.right.flatMap(_.downField("include_paths").as[Seq[String]]).toOption.getOrElse(Seq.empty)
val configPath = providedConfig.right.flatMap(_.downField("config").downField("config").as[String]).toOption.getOrElse(defaultStyleConfigurationPath)

val config = ScalastyleCodeClimateConfiguration(configPath, includePaths, Seq.empty)

val ccPrinter = new CodeClimateIssuePrinter(programArgs.workspace_path, Console.out)

ScalaStyleRunner.runCheckstyle(programArgs.workspace_path, config) foreach ccPrinter.printIssue
case None => // it will print the error
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
package org.codeclimate.scalastyle

import java.io.{File, PrintStream}

import com.typesafe.config.ConfigFactory
import io.circe.Printer
import io.circe.generic.auto._
import io.circe.syntax._
import org.scalastyle.{FileSpec, Message, MessageHelper, StyleError}

import scala.collection.JavaConverters._

class CodeClimateIssuePrinter(workspacePath: String, ps: PrintStream) {
private val basePath = new File(workspacePath).toPath
private val printer = Printer.noSpaces.copy(dropNullKeys = true)

private val messageHelper = new MessageHelper(ConfigFactory.load())

def printIssue[T <: FileSpec](msg: Message[T]): Unit = msg match {
case se: StyleError[FileSpec] =>
val errPosition = Position(se.lineNumber.getOrElse(0), se.column.getOrElse(0))
val filePath = Option(se.fileSpec.name)
.map(pathname => basePath.relativize(new File(pathname).toPath))
.map(_.toString)
.getOrElse(se.fileSpec.name)

val location = Location(path = filePath, positions = LinePosition(
errPosition, errPosition
))
val msg: String = se.customMessage.orElse {
Some(messageHelper.message(se.key, se.args))
}.getOrElse("Error message not provided")
val issue = Issue(location = location,
description = String.format(msg, se.args.asJava),
check_name = Some(se.clazz.getName),
categories = Seq("Style"),
severity = Some("major")
)
val jsonStr = printer.pretty(issue.asJson)
ps.print(jsonStr)
ps.print("\0")
case _ => // ignore
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
package org.codeclimate.scalastyle

import java.io.File

import org.scalastyle._

/**
* Computes files and run ScalastyleChecker against them.
*/
private object ScalaStyleRunner {
def runCheckstyle(workspacePath: String, ccConfig: ScalastyleCodeClimateConfiguration): Seq[Message[FileSpec]] = {
val paths = if (ccConfig.include_paths.isEmpty) {
Seq(workspacePath)
} else {
ccConfig.include_paths.map(include => s"$workspacePath/$include")
}
val files = Directory.getFiles(None, paths.map(new File(_)), excludedFiles = ccConfig.exclude_paths)

val scalastyleConfig = ScalastyleConfiguration.readFromXml(ccConfig.config)
new ScalastyleChecker(None).checkFiles(scalastyleConfig, files)
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
package org.codeclimate

package object scalastyle {
private[this] val DEFAULT_REMEDIAION_POINTS = 50000

case class ScalastyleCodeClimateConfiguration(
config: String,
include_paths: Seq[String] = Seq.empty,
exclude_paths: Seq[String] = Seq.empty
)

sealed trait IssueSchema extends Product with Serializable
case class Position(line: Int, column: Int) extends IssueSchema
case class LinePosition(begin: Position, end: Position) extends IssueSchema
case class Location(path: String, positions: LinePosition) extends IssueSchema
case class Issue(location: Location, description: String, check_name: Option[String] = None,
severity: Option[String] = None, remediation_points: Option[Int] = Some(DEFAULT_REMEDIAION_POINTS),
fingerprint: Option[String] = None, `type`: String = "issue", categories: Seq[String] = Seq.empty
) extends IssueSchema
}
12 changes: 12 additions & 0 deletions engine-core/src/test/resources/apackage/TestFile.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
package apackage

class TestFile {
val MyVariable = ""

def foobar(s: String) = {
var a = 1
lazy var b = a
a = 2
b
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
package apackage

class TestFileToIgnore {
val pi = Some(3.14)
} // it should at least report missing new line
Original file line number Diff line number Diff line change
@@ -1,30 +1,11 @@
<scalastyle commentFilter="enabled">
<name>Scalastyle standard configuration</name>
<check level="warning" class="org.scalastyle.file.FileTabChecker" enabled="true"></check>
<check level="warning" class="org.scalastyle.file.FileLengthChecker" enabled="true">
<check level="warning" class="org.scalastyle.file.FileTabChecker" enabled="false"></check>
<check level="warning" class="org.scalastyle.file.FileLengthChecker" enabled="false">
<parameters>
<parameter name="maxFileLength"><![CDATA[800]]></parameter>
</parameters>
</check>
<check level="warning" class="org.scalastyle.file.HeaderMatchesChecker" enabled="true">
<parameters>
<parameter name="header"><![CDATA[// Copyright (C) 2011-2012 the original author or authors.
// See the LICENCE.txt file distributed with this work for additional
// information regarding copyright ownership.
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.]]></parameter>
</parameters>
</check>
<check level="warning" class="org.scalastyle.scalariform.SpacesAfterPlusChecker" enabled="true"></check>
<check level="warning" class="org.scalastyle.file.WhitespaceEndOfLineChecker" enabled="true"></check>
<check level="warning" class="org.scalastyle.scalariform.SpacesBeforePlusChecker" enabled="true"></check>
Expand Down Expand Up @@ -139,4 +120,4 @@
</parameters>
</check>
<check level="warning" class="org.scalastyle.scalariform.ImportGroupingChecker" enabled="true"></check>
</scalastyle>
</scalastyle>
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
package org.codeclimate.scalastyle

import org.scalastyle.StyleError
import org.scalatest.Matchers

class CodeClimateRunnerTest extends org.scalatest.FreeSpec with Matchers {
val workspacePath = "engine-core"

val codeClimateConfiguration = ScalastyleCodeClimateConfiguration(
config = "engine-core/src/test/resources/scalastyle_config.xml",
include_paths = Seq("src/test/resources")
)

"CodeClimateEngine" - {
"should call sclacheck and produce style errors for both files in apackage" in {
val msgs = ScalaStyleRunner.runCheckstyle(workspacePath, codeClimateConfiguration)

msgs should not be empty

val styleErrors = msgs.flatMap {
case se: StyleError[_] => Seq(se)
case _ => Seq.empty
}

styleErrors should have size 3 // pre-computed number of issues
}

"should ignore files specified in `exclude_paths`" in {
val msgs = ScalaStyleRunner.runCheckstyle(workspacePath, codeClimateConfiguration.copy(
exclude_paths = Seq("TestFileToIgnore"))
)

val files = msgs.flatMap {
case se: StyleError[_] => Seq(se.fileSpec.name)
case _ => Seq.empty
}

files filter(_.contains("TestFileToIgnore")) shouldBe empty
}
}
}
Loading