Skip to content

Commit

Permalink
Merge pull request #6914 from http4s/non-validating-copy-constructor
Browse files Browse the repository at this point in the history
Remove nonValidatingCopyConstructor linter
  • Loading branch information
rossabaker committed Jan 8, 2023
2 parents 4bf37db + 4966fd0 commit 54bec99
Show file tree
Hide file tree
Showing 11 changed files with 6 additions and 38 deletions.
1 change: 0 additions & 1 deletion core/shared/src/main/scala/org/http4s/Charset.scala
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,6 @@ import java.util.HashMap
import java.util.Locale
import scala.jdk.CollectionConverters._

// scalafix:off Http4sGeneralLinters; bincompat until 1.0
final case class Charset private[http4s] (nioCharset: NioCharset) extends Renderable {
def withQuality(q: QValue): CharsetRange.Atom = CharsetRange.Atom(this, q)
def toRange: CharsetRange.Atom = withQuality(QValue.One)
Expand Down
2 changes: 0 additions & 2 deletions core/shared/src/main/scala/org/http4s/HttpVersion.scala
Original file line number Diff line number Diff line change
Expand Up @@ -40,11 +40,9 @@ import org.http4s.util._
* [[https://httpwg.org/http-core/draft-ietf-httpbis-semantics-latest.html#protocol.version
* HTTP Semantics, Protocol Versioning]]
*/
// scalafix:off Http4sGeneralLinters.nonValidatingCopyConstructor; bincompat until 1.0
final case class HttpVersion private (major: Int, minor: Int)
extends Renderable
with Ordered[HttpVersion] {
// scalafix:on

/** Renders as an HTTP/1.1 string
*
Expand Down
2 changes: 1 addition & 1 deletion core/shared/src/main/scala/org/http4s/Uri.scala
Original file line number Diff line number Diff line change
Expand Up @@ -609,7 +609,7 @@ object Uri extends UriPlatform {
*
* @see [[https://datatracker.ietf.org/doc/html/rfc3986#section-3.2.1 RFC 3986, Section 3.2.1, User Information]]
*/
final case class UserInfo private ( // scalafix:ok Http4sGeneralLinters.nonValidatingCopyConstructor; bincompat until 1.0
final case class UserInfo private (
username: String,
password: Option[String],
) extends Ordered[UserInfo] {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ object `Access-Control-Max-Age` {
*
* @param age age of the response (in seconds)
*/
final case class Cache private (age: Long) extends `Access-Control-Max-Age` { // scalafix:ok Http4sGeneralLinters.nonValidatingCopyConstructor; bincompat until 1.0
final case class Cache private (age: Long) extends `Access-Control-Max-Age` {
def duration: Option[FiniteDuration] = Try(age.seconds).toOption
def unsafeDuration: FiniteDuration = age.seconds
}
Expand Down
2 changes: 1 addition & 1 deletion core/shared/src/main/scala/org/http4s/headers/Age.scala
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ object Age {
*
* @param age age of the response
*/
final case class Age private (age: Long) { // scalafix:ok Http4sGeneralLinters.nonValidatingCopyConstructor; bincompat until 1.0
final case class Age private (age: Long) {
def duration: Option[FiniteDuration] = Try(age.seconds).toOption

def unsafeDuration: FiniteDuration = age.seconds
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ object `Content-Type` {
*
* [[https://datatracker.ietf.org/doc/html/rfc7231#section-3.1.1.5 RFC-7231]]
*/
final case class `Content-Type` private (mediaType: MediaType, charset: Option[Charset]) { // scalafix:ok; private for API ergonomics, not correctness
final case class `Content-Type` private (mediaType: MediaType, charset: Option[Charset]) {
def withMediaType(mediaType: MediaType): `Content-Type` =
if (mediaType != this.mediaType) copy(mediaType = mediaType) else this
def withCharset(charset: Charset): `Content-Type` =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,4 +50,4 @@ object `Sec-WebSocket-Version` {

final case class `Sec-WebSocket-Version` private (
version: Long
) // scalafix:ok Http4sGeneralLinters.nonValidatingCopyConstructor; bincompat until 1.0
)
Original file line number Diff line number Diff line change
Expand Up @@ -155,8 +155,6 @@ object CharPredicate {

private def unmaskable(c: Char) = c >= 128

// scalafix:off Http4sGeneralLinters.nonValidatingCopyConstructor; bincompat until 1.0

// efficient handling of 7bit-ASCII chars
final case class MaskBased private[CharPredicate] (lowMask: Long, highMask: Long)
extends CharPredicate {
Expand Down Expand Up @@ -351,6 +349,4 @@ object CharPredicate {

override def toString(): String = "CharPredicate.General@" + System.identityHashCode(this)
}

// scalafix:on
}
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,3 @@ class Dog extends Animal// assert: Http4sGeneralLinters.leakingSealedHierarchy

sealed abstract class Plant
trait Tree extends Plant// assert: Http4sGeneralLinters.leakingSealedHierarchy

package pkg {
final case class Foo private[pkg](v: String)// assert: Http4sGeneralLinters.nonValidatingCopyConstructor
}
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,3 @@ class Dog extends Animal

sealed abstract class Plant
trait Tree extends Plant

package pkg {
final case class Foo private[pkg](v: String)
}
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ import scala.meta._

class GeneralLinters extends SemanticRule("Http4sGeneralLinters") {
override def fix(implicit doc: SemanticDocument): Patch =
noFinalObject + noNonFinalCaseClass + leakingSealedHierarchy + nonValidatingCopyConstructor
noFinalObject + noNonFinalCaseClass + leakingSealedHierarchy

def noFinalObject(implicit doc: SemanticDocument) =
doc.tree.collect { case o @ Defn.Object(mods, _, _) =>
Expand Down Expand Up @@ -64,14 +64,6 @@ class GeneralLinters extends SemanticRule("Http4sGeneralLinters") {
doCheck(t, mods, inits)
}.asPatch
}

def nonValidatingCopyConstructor(implicit doc: SemanticDocument) =
doc.tree.collect {
case c @ Defn.Class(mods, _, _, Ctor.Primary(ctorMods, _, _), _)
if mods.exists(_.is[Mod.Case]) && ctorMods
.exists(_.is[Mod.Private]) && !mods.exists(_.is[Mod.Abstract]) =>
Patch.lint(NonValidatingCopyConstructor(c))
}.asPatch
}

final case class CaseClassWithoutAccessModifier(c: Defn.Class) extends Diagnostic {
Expand All @@ -89,12 +81,3 @@ final case class LeakingSealedHierarchy(t: Tree) extends Diagnostic {

override def categoryID: String = "leakingSealedHierarchy"
}

final case class NonValidatingCopyConstructor(c: Defn.Class) extends Diagnostic {
override def message: String =
"Case classes with private constructors should be abstract to prevent exposing a non-validating copy constructor"

override def position: Position = c.pos

override def categoryID: String = "nonValidatingCopyConstructor"
}

0 comments on commit 54bec99

Please sign in to comment.