Skip to content

Commit

Permalink
Fixes scalastyle#16 Custom messages for a rule
Browse files Browse the repository at this point in the history
Done through the configuration xml, via an element <customMessage>
  • Loading branch information
matthewfarwell committed Apr 9, 2012
1 parent fe82e0d commit 5c44afc
Show file tree
Hide file tree
Showing 7 changed files with 102 additions and 20 deletions.
11 changes: 10 additions & 1 deletion README.md
Expand Up @@ -34,7 +34,6 @@ Want to contribute? Great! Look at the wiki for potential rules to implement, or
6. Create an [Issue](https://github.com/scalastyle/scalastyle/issues) with a link to your branch 6. Create an [Issue](https://github.com/scalastyle/scalastyle/issues) with a link to your branch
7. Enjoy a coffee and wait 7. Enjoy a coffee and wait



Comment filters Comment filters
--------------- ---------------


Expand All @@ -49,6 +48,16 @@ You can also switch off checking for a particular rule by specifying the id of t
var foobar = 134 var foobar = 134
// scalastyle:on magic.number // scalastyle:on magic.number


Custom messages
---------------

Messages are defined within the scalastyle_messages.properties. If however, you wish to have a custom error message for a particular rule, then
you can do so by defining a customMessage element in the configuration, such as:

<check level="warning" class="org.scalastyle.scalariform.MagicNumberChecker" enabled="true">
<customMessage>Please don't use magic numbers</customMessage>
</check>

Implemented Rules Implemented Rules
----------------- -----------------


Expand Down
15 changes: 9 additions & 6 deletions src/main/scala/org/scalastyle/Checker.scala
Expand Up @@ -74,7 +74,7 @@ object Checker {
case None => List[CommentFilter]() case None => List[CommentFilter]()
} }


classes.flatMap(cc => newInstance(cc.className, cc.level, cc.parameters)).map(c => c match { classes.flatMap(cc => newInstance(cc.className, cc.level, cc.parameters, cc.customMessage)).map(c => c match {
case c: FileChecker => c.verify(file, c.level, lines, lines) case c: FileChecker => c.verify(file, c.level, lines, lines)
case c: ScalariformChecker => scalariformAst match { case c: ScalariformChecker => scalariformAst match {
case Some(ast) => c.verify(file, c.level, ast.ast, lines) case Some(ast) => c.verify(file, c.level, ast.ast, lines)
Expand All @@ -93,12 +93,13 @@ object Checker {
} }
} }


def newInstance(name: String, level: Level, parameters: Map[String, String]): Option[Checker[_]] = { def newInstance(name: String, level: Level, parameters: Map[String, String], customMessage: Option[String]): Option[Checker[_]] = {
try { try {
val clazz = Class.forName(name).asInstanceOf[Class[Checker[_]]] val clazz = Class.forName(name).asInstanceOf[Class[Checker[_]]]
val c: Checker[_] = clazz.getConstructor().newInstance().asInstanceOf[Checker[_]] val c: Checker[_] = clazz.getConstructor().newInstance().asInstanceOf[Checker[_]]
c.setParameters(parameters) c.setParameters(parameters)
c.setLevel(level) c.setLevel(level)
c.setCustomMessage(customMessage)
Some(c) Some(c)
} catch { } catch {
case e: Exception => { case e: Exception => {
Expand All @@ -113,9 +114,11 @@ trait Checker[A] {
val errorKey: String; val errorKey: String;
var parameters = Map[String, String](); var parameters = Map[String, String]();
var level: Level = WarningLevel; var level: Level = WarningLevel;
var customMessage: Option[String] = None;


def setParameters(parameters: Map[String, String]) = this.parameters = parameters; def setParameters(parameters: Map[String, String]) = this.parameters = parameters;
def setLevel(level: Level) = this.level = level; def setLevel(level: Level) = this.level = level;
def setCustomMessage(customMessage: Option[String]) = this.customMessage = customMessage
def getInt(parameter: String, defaultValue: Int) = Integer.parseInt(parameters.getOrElse(parameter, "" + defaultValue)) def getInt(parameter: String, defaultValue: Int) = Integer.parseInt(parameters.getOrElse(parameter, "" + defaultValue))
def getString(parameter: String, defaultValue: String) = parameters.getOrElse(parameter, defaultValue) def getString(parameter: String, defaultValue: String) = parameters.getOrElse(parameter, defaultValue)


Expand All @@ -131,10 +134,10 @@ trait Checker[A] {
} }


p2 match { p2 match {
case PositionError(position, args) => StyleError(file, this.getClass(), errorKey, level, args) case PositionError(position, args) => StyleError(file, this.getClass(), errorKey, level, args, customMessage = customMessage)
case FileError(args) => StyleError(file, this.getClass(), errorKey, level, args, None, None) case FileError(args) => StyleError(file, this.getClass(), errorKey, level, args, None, None, customMessage)
case LineError(line, args) => StyleError(file, this.getClass(), errorKey, level, args, Some(line), None) case LineError(line, args) => StyleError(file, this.getClass(), errorKey, level, args, Some(line), None, customMessage)
case ColumnError(line, column, args) => StyleError(file, this.getClass(), errorKey, level, args, Some(line), Some(column)) case ColumnError(line, column, args) => StyleError(file, this.getClass(), errorKey, level, args, Some(line), Some(column), customMessage)
} }
} }


Expand Down
5 changes: 3 additions & 2 deletions src/main/scala/org/scalastyle/Message.scala
Expand Up @@ -63,8 +63,9 @@ case class StartFile[+T <: FileSpec](fileSpec: T) extends Message[T]
case class EndFile[+T <: FileSpec](fileSpec: T) extends Message[T] case class EndFile[+T <: FileSpec](fileSpec: T) extends Message[T]


case class StyleError[+T <: FileSpec](fileSpec: T, clazz: Class[_ <: Checker[_]], key: String, case class StyleError[+T <: FileSpec](fileSpec: T, clazz: Class[_ <: Checker[_]], key: String,
level: Level, args: List[String], lineNumber: Option[Int] = None, column: Option[Int] = None) extends Message[T] { level: Level, args: List[String], lineNumber: Option[Int] = None,
override def toString() = "StyleError key=" + key + " args=" + args + " lineNumber=" + lineNumber + " column=" + column column: Option[Int] = None, customMessage: Option[String] = None) extends Message[T] {
override def toString() = "StyleError key=" + key + " args=" + args + " lineNumber=" + lineNumber + " column=" + column + " customMessage=" + customMessage
} }
case class StyleException[+T <: FileSpec](fileSpec: T, clazz: Option[Class[_ <: Checker[_]]], message: String, case class StyleException[+T <: FileSpec](fileSpec: T, clazz: Option[Class[_ <: Checker[_]]], message: String,
stacktrace: String, lineNumber: Option[Int] = None, column: Option[Int] = None) extends Message[T] stacktrace: String, lineNumber: Option[Int] = None, column: Option[Int] = None) extends Message[T]
Expand Down
15 changes: 11 additions & 4 deletions src/main/scala/org/scalastyle/Output.scala
Expand Up @@ -17,6 +17,7 @@
package org.scalastyle package org.scalastyle


trait Output[T <: FileSpec] { trait Output[T <: FileSpec] {
protected[this] val messageHelper = new MessageHelper(this.getClass().getClassLoader())
private var errors = 0 private var errors = 0
private var warnings = 0 private var warnings = 0
private var files = 0 private var files = 0
Expand All @@ -31,28 +32,34 @@ trait Output[T <: FileSpec] {
case EndWork() => case EndWork() =>
case StartFile(file) => files += 1 case StartFile(file) => files += 1
case EndFile(file) => case EndFile(file) =>
case StyleError(file, clazz, key, level, args, line, column) => level match { case StyleError(file, clazz, key, level, args, line, column, customMessage) => level match {
case WarningLevel => warnings += 1 case WarningLevel => warnings += 1
case _ => errors += 1 case _ => errors += 1
} }
case StyleException(file, clazz, message, stacktrace, line, column) => errors += 1 case StyleException(file, clazz, message, stacktrace, line, column) => errors += 1
} }


def findMessage(clazz: Class[_], key: String, args: List[String], customMessage: Option[String]): String = {
customMessage match {
case Some(s) => s
case None => messageHelper.message(clazz.getClassLoader(), key, args)
}
}

def message(m: Message[T]): Unit def message(m: Message[T]): Unit
} }


case class OutputResult(files: Int, errors: Int, warnings: Int) case class OutputResult(files: Int, errors: Int, warnings: Int)


class TextOutput[T <: FileSpec](verbose: Boolean = false, quiet: Boolean = false) extends Output[T] { class TextOutput[T <: FileSpec](verbose: Boolean = false, quiet: Boolean = false) extends Output[T] {
private val messageHelper = new MessageHelper(getClass().getClassLoader())
override def message(m: Message[T]) = m match { override def message(m: Message[T]) = m match {
case StartWork() => if (verbose) println("Starting scalastyle") case StartWork() => if (verbose) println("Starting scalastyle")
case EndWork() => case EndWork() =>
case StartFile(file) => if (verbose) println("start file " + file) case StartFile(file) => if (verbose) println("start file " + file)
case EndFile(file) => if (verbose) println("end file " + file) case EndFile(file) => if (verbose) println("end file " + file)
case StyleError(file, clazz, key, level, args, line, column) => { case StyleError(file, clazz, key, level, args, line, column, customMessage) => {
println(messageHelper.text(level.name) + print("file", file.name) + println(messageHelper.text(level.name) + print("file", file.name) +
print("message", messageHelper.message(clazz.getClassLoader(), key, args)) + print("message", findMessage(clazz, key, args, customMessage)) +
print("line", line) + print("column", column)) print("line", line) + print("column", column))
} }
case StyleException(file, clazz, message, stacktrace, line, column) => { case StyleException(file, clazz, message, stacktrace, line, column) => {
Expand Down
20 changes: 16 additions & 4 deletions src/main/scala/org/scalastyle/ScalastyleConfiguration.scala
Expand Up @@ -19,6 +19,7 @@ package org.scalastyle
import scala.xml.XML import scala.xml.XML
import scala.xml.Elem; import scala.xml.Elem;
import scala.xml.Node; import scala.xml.Node;
import scala.xml.NodeSeq;


object Level { object Level {
def apply(s: String) = s match { def apply(s: String) = s match {
Expand All @@ -42,7 +43,7 @@ sealed abstract class ParameterType(val name: String)
case object IntegerType extends ParameterType("integer") case object IntegerType extends ParameterType("integer")
case object StringType extends ParameterType("string") case object StringType extends ParameterType("string")


case class ConfigurationChecker(className: String, level: Level, enabled: Boolean, parameters: Map[String, String]) case class ConfigurationChecker(className: String, level: Level, enabled: Boolean, parameters: Map[String, String], customMessage: Option[String])


object ScalastyleConfiguration { object ScalastyleConfiguration {
def readFromXml(file: String): ScalastyleConfiguration = { def readFromXml(file: String): ScalastyleConfiguration = {
Expand All @@ -57,26 +58,37 @@ object ScalastyleConfiguration {
val className = node.attribute("class").get.text val className = node.attribute("class").get.text
val level = Level(node.attribute("level").get.text) val level = Level(node.attribute("level").get.text)
val enabled = node.attribute("enabled").getOrElse(scala.xml.Text("false")).text.toLowerCase() == "true" val enabled = node.attribute("enabled").getOrElse(scala.xml.Text("false")).text.toLowerCase() == "true"
val ns = (node \\ "customMessage")
val customMessage = if (ns.size == 0) None else (Some(ns(0).text))


ConfigurationChecker(className, level, enabled, (node \\ "parameters" \\ "parameter").map(e => { ConfigurationChecker(className, level, enabled, (node \\ "parameters" \\ "parameter").map(e => {
val attributeValue = e.attribute("value") val attributeValue = e.attribute("value")
val value = if (attributeValue.isDefined) attributeValue.get.text else e.text val value = if (attributeValue.isDefined) attributeValue.get.text else e.text
(e.attribute("name").head.text -> value) (e.attribute("name").head.text -> value)
}).toMap) }).toMap, customMessage)
} }


private[this] def toCDATA(s: String) = scala.xml.Unparsed("<![CDATA[" + s + "]]>")

def toXml(scalastyleConfiguration: ScalastyleConfiguration): scala.xml.Elem = { def toXml(scalastyleConfiguration: ScalastyleConfiguration): scala.xml.Elem = {
val elements = scalastyleConfiguration.checks.map(c => { val elements = scalastyleConfiguration.checks.map(c => {
val parameters = if (c.parameters.size > 0) { val parameters = if (c.parameters.size > 0) {
val ps = c.parameters.map( p => { val ps = c.parameters.map( p => {
val text = scala.xml.Unparsed("<![CDATA[" + p._2 + "]]>") val text = toCDATA(p._2)
<parameter name={p._1}>{text}</parameter> <parameter name={p._1}>{text}</parameter>
}) })
<parameters>{ps}</parameters> <parameters>{ps}</parameters>
} else { } else {
scala.xml.Null scala.xml.Null
} }
<check class={c.className} level={c.level.name} enabled={if (c.enabled) "true" else "false"}>{parameters}</check> val customMessage = c.customMessage match {
case Some(s) => {
val text = toCDATA(s)
<customMessage>{text}</customMessage>
}
case None => scala.xml.Null
}
<check class={c.className} level={c.level.name} enabled={if (c.enabled) "true" else "false"}>{customMessage}{parameters}</check>
}) })


<scalastyle><name>{scalastyleConfiguration.name}</name>{elements}</scalastyle> <scalastyle><name>{scalastyleConfiguration.name}</name>{elements}</scalastyle>
Expand Down
47 changes: 47 additions & 0 deletions src/test/scala/org/scalastyle/CustomMessageTest.scala
@@ -0,0 +1,47 @@
// 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.

package org.scalastyle

// scalastyle:off magic.number

import org.scalatest.junit.AssertionsForJUnit
import org.junit.Assert.assertEquals
import org.junit.Assert.assertTrue
import org.junit.Test
import _root_.scalariform.lexer.HiddenTokenInfo
import _root_.scalariform.lexer.Tokens._
import org.scalastyle.file.CheckerTest
import org.scalastyle.file.FileLengthChecker

class CustomMessageTest extends CheckerTest {
val key = "file.size.limit"
val classUnderTest = classOf[FileLengthChecker]
val message = Some("custom")

@Test def testOne() = {
val source = """
package foobar
object Foobar {
}
object Barbar {
}
""";

assertErrors(List(fileError(List("5"), message)), source, Map("maxFileLength" -> "5"), message)
}
}
9 changes: 6 additions & 3 deletions src/test/scala/org/scalastyle/file/CheckerTest.scala
Expand Up @@ -34,11 +34,14 @@ trait CheckerTest {
def name() = "" def name() = ""
} }


protected def assertErrors[T <: FileSpec](list: List[Message[T]], source: String, params: Map[String, String] = Map()) = { protected def assertErrors[T <: FileSpec](list: List[Message[T]], source: String, params: Map[String, String] = Map(),
assertEquals(list, Checker.verifySource(List(ConfigurationChecker(classUnderTest.getName(), WarningLevel, true, params)), NullFileSpec, source)) customMessage: Option[String] = None) = {
assertEquals(list, Checker.verifySource(List(ConfigurationChecker(classUnderTest.getName(), WarningLevel,
true, params, customMessage)), NullFileSpec, source))
} }


protected def fileError(args: List[String] = List()) = StyleError(NullFileSpec, classUnderTest, key, WarningLevel, args, None, None) protected def fileError(args: List[String] = List(), customMessage: Option[String] = None) =
StyleError(NullFileSpec, classUnderTest, key, WarningLevel, args, None, None, customMessage)
protected def lineError(line: Int, args: List[String] = List()) = StyleError(NullFileSpec, classUnderTest, key, WarningLevel, args, Some(line), None) protected def lineError(line: Int, args: List[String] = List()) = StyleError(NullFileSpec, classUnderTest, key, WarningLevel, args, Some(line), None)
protected def columnError(line: Int, column: Int, args: List[String] = List()) = protected def columnError(line: Int, column: Int, args: List[String] = List()) =
StyleError(NullFileSpec, classUnderTest, key, WarningLevel, args, Some(line), Some(column)) StyleError(NullFileSpec, classUnderTest, key, WarningLevel, args, Some(line), Some(column))
Expand Down

0 comments on commit 5c44afc

Please sign in to comment.