Skip to content

Commit 725e46f

Browse files
cb372cb372
authoredMar 12, 2020
Fix a few warnings (#843)
* Inline an implicit class and delete dead code * Fix some warnings in serviceImpl, disable others * Fix a few more warnings * Fix a bunch of warnings in the tests * Fix false-positive compiler warnings in macro-generated code * Disable false-positive compiler warnings for Scala 2.12
·
v0.34.0v0.22.0
1 parent 67684e0 commit 725e46f

File tree

14 files changed

+78
-107
lines changed

14 files changed

+78
-107
lines changed
 

‎modules/channel/src/test/scala/higherkindness/mu/rpc/channel/metrics/MonitoringChannelInterceptorTests.scala

+1-1
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@
1616

1717
package higherkindness.mu.rpc.channel.metrics
1818

19-
import cats.effect.{Clock, IO, Resource}
19+
import cats.effect.{IO, Resource}
2020
import cats.syntax.apply._
2121
import higherkindness.mu.rpc.common.{A => _, _}
2222
import higherkindness.mu.rpc.common.util.FakeClock

‎modules/common/src/test/scala/higherkindness/mu/rpc/common/RpcBaseTestSuite.scala

+2-4
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,6 @@ import cats.data.Kleisli
2020
import org.scalactic.Prettifier
2121
import org.scalamock.scalatest.MockFactory
2222
import org.scalatest._
23-
import scala.compat.Platform
2423
import scala.io._
2524
import org.scalatest.matchers.should.Matchers
2625
import org.scalatest.wordspec.AnyWordSpec
@@ -36,9 +35,8 @@ trait RpcBaseTestSuite extends AnyWordSpec with Matchers with OneInstancePerTest
3635

3736
implicit val prettifier: Prettifier = Prettifier {
3837
case x => // includes null
39-
Platform.EOL + Prettifier.default(
40-
x
41-
) // initial linebreak makes expected/actual results line up nicely
38+
// initial linebreak makes expected/actual results line up nicely
39+
System.lineSeparator + Prettifier.default(x)
4240
}
4341

4442
// delegating the check to a def gets its name used in the cancellation message (cleaner than the boolean comparison result)

‎modules/health-check-unary/src/main/scala/higherkindness/mu/rpc/healthcheck/unary/handler/HealthServiceImpl.scala

+6-2
Original file line numberDiff line numberDiff line change
@@ -42,13 +42,17 @@ abstract class AbstractHealthService[F[_]: Functor](
4242
def clearStatus(service: HealthCheck): F[Unit] =
4343
checkStatus.update(_ - service.nameService)
4444

45-
def checkAll(empty: Empty.type): F[AllStatus] =
45+
def checkAll(empty: Empty.type): F[AllStatus] = {
46+
val _ = empty // just to suppress compiler warning about unused parameter
4647
checkStatus.get
4748
.map(_.map(p => HealthStatus(new HealthCheck(p._1), p._2)).toList)
4849
.map(AllStatus)
50+
}
4951

50-
def cleanAll(empty: Empty.type): F[Unit] =
52+
def cleanAll(empty: Empty.type): F[Unit] = {
53+
val _ = empty // just to suppress compiler warning about unused parameter
5154
checkStatus.set(Map.empty[String, ServerStatus])
55+
}
5256

5357
}
5458

‎modules/http/src/main/scala/higherkindness/mu/http/implicits.scala

+1-1
Original file line numberDiff line numberDiff line change
@@ -132,5 +132,5 @@ final case class UnexpectedError(className: String, msg: Option[String])
132132
with NoStackTrace
133133

134134
final case class ResponseError(status: Status, msg: Option[String] = None)
135-
extends RuntimeException(status + msg.fold("")(": " + _))
135+
extends RuntimeException(status.toString + msg.fold("")(": " + _))
136136
with NoStackTrace

‎modules/internal/fs2/src/main/scala/higherkindness/mu/server/fs2Calls.scala

+3
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,9 @@ import org.lyranthe.fs2_grpc.java_runtime.server.Fs2ServerCallHandler
2525

2626
object fs2Calls {
2727

28+
// TODO Support compression. It wasn't supported in fs2-grpc
29+
// at the time this code was added (#477), but it is now.
30+
2831
def unaryMethod[F[_]: ConcurrentEffect, Req, Res](
2932
f: Req => F[Res],
3033
maybeCompression: Option[String]

‎modules/internal/src/main/scala/higherkindness/mu/rpc/internal/serviceImpl.scala

+41-12
Original file line numberDiff line numberDiff line change
@@ -33,8 +33,8 @@ object serviceImpl {
3333
def getInner: Option[Tree] = inner
3434
def safeInner: Tree = inner.getOrElse(tpe)
3535
def safeType: Tree = tpe match {
36-
case tq"$s[..$tpts]" if isStreaming => tpts.last
37-
case other => other
36+
case tq"${s @ _}[..$tpts]" if isStreaming => tpts.last
37+
case other => other
3838
}
3939
def flatName: String = safeInner.toString
4040

@@ -51,12 +51,12 @@ object serviceImpl {
5151
}
5252
object TypeTypology {
5353
def apply(t: Tree): TypeTypology = t match {
54-
case tq"Observable[..$tpts]" => MonixObservableTpe(t, tpts.headOption)
55-
case tq"_root_.monix.reactive.Observable[..$tpts]" => MonixObservableTpe(t, tpts.headOption)
56-
case tq"Stream[$carrier, ..$tpts]" => Fs2StreamTpe(t, tpts.headOption)
57-
case tq"_root_.fs2.Stream[$carrier, ..$tpts]" => Fs2StreamTpe(t, tpts.headOption)
58-
case tq"Empty.type" => EmptyTpe(t)
59-
case tq"$carrier[..$tpts]" => UnaryTpe(t, tpts.headOption)
54+
case tq"Observable[..$tpts]" => MonixObservableTpe(t, tpts.headOption)
55+
case tq"_root_.monix.reactive.Observable[..$tpts]" => MonixObservableTpe(t, tpts.headOption)
56+
case tq"Stream[${carrier @ _}, ..$tpts]" => Fs2StreamTpe(t, tpts.headOption)
57+
case tq"_root_.fs2.Stream[${carrier @ _}, ..$tpts]" => Fs2StreamTpe(t, tpts.headOption)
58+
case tq"Empty.type" => EmptyTpe(t)
59+
case tq"${carrier @ _}[..$tpts]" => UnaryTpe(t, tpts.headOption)
6060
}
6161
}
6262
case class EmptyTpe(tpe: Tree) extends TypeTypology(tpe, None)
@@ -285,6 +285,34 @@ object serviceImpl {
285285
..$nonRpcDefs
286286
}""".supressWarts("DefaultArguments")
287287

288+
/*
289+
* When you write an anonymous parameter in an anonymous function that
290+
* ignores the parameter, e.g. `List(1, 2, 3).map(_ => "hello")` the
291+
* -Wunused:params scalac flag does not warn you about it. That's
292+
* because the compiler attaches a `NoWarnAttachment` to the tree for
293+
* the parameter.
294+
*
295+
* But if you write the same thing in a quasiquote inside a macro, the
296+
* attachment does not get added, so you get false-positive compiler
297+
* warnings at the macro use site like: "parameter value x$2 in anonymous
298+
* function is never used".
299+
*
300+
* (The parameter needs a name, even though the function doesn't
301+
* reference it, so `_` gets turned into a fresh name e.g. `x$2`. The
302+
* same thing happens even if you're not in a macro.)
303+
*
304+
* I'd say this is a bug in Scala. We work around it by manually adding
305+
* the attachment.
306+
*/
307+
private def anonymousParam: ValDef = {
308+
val tree: ValDef = q"{(_) => 1}".vparams.head
309+
c.universe.internal.updateAttachment(
310+
tree,
311+
c.universe.asInstanceOf[scala.reflect.internal.StdAttachments].NoWarnAttachment
312+
)
313+
tree
314+
}
315+
288316
val client: DefDef =
289317
q"""
290318
def client[$F_](
@@ -296,7 +324,7 @@ object serviceImpl {
296324
_root_.cats.effect.Resource.make {
297325
new _root_.higherkindness.mu.rpc.channel.ManagedChannelInterpreter[$F](channelFor, channelConfigList).build
298326
}(channel => CE.void(CE.delay(channel.shutdown()))).flatMap(ch =>
299-
_root_.cats.effect.Resource.make[F, $serviceName[$F]](CE.delay(new $Client[$F](ch, options)))(_ => CE.unit))
327+
_root_.cats.effect.Resource.make[F, $serviceName[$F]](CE.delay(new $Client[$F](ch, options)))($anonymousParam=> CE.unit))
300328
""".supressWarts("DefaultArguments")
301329

302330
val clientFromChannel: DefDef =
@@ -306,7 +334,7 @@ object serviceImpl {
306334
options: _root_.io.grpc.CallOptions = _root_.io.grpc.CallOptions.DEFAULT
307335
)(implicit ..$classImplicits): _root_.cats.effect.Resource[$F, $serviceName[$F]] = _root_.cats.effect.Resource.make(channel)(channel =>
308336
CE.void(CE.delay(channel.shutdown()))).flatMap(ch =>
309-
_root_.cats.effect.Resource.make[$F, $serviceName[$F]](CE.delay(new $Client[$F](ch, options)))(_ => CE.unit))
337+
_root_.cats.effect.Resource.make[$F, $serviceName[$F]](CE.delay(new $Client[$F](ch, options)))($anonymousParam => CE.unit))
310338
""".supressWarts("DefaultArguments")
311339

312340
val unsafeClient: DefDef =
@@ -565,8 +593,9 @@ object serviceImpl {
565593
}
566594

567595
val operations: List[HttpOperation] = for {
568-
d <- rpcDefs.collect { case x if findAnnotation(x.mods, "http").isDefined => x }
569-
args <- findAnnotation(d.mods, "http").collect({ case Apply(_, args) => args }).toList
596+
d <- rpcDefs.collect { case x if findAnnotation(x.mods, "http").isDefined => x }
597+
// TODO not sure what the following line is doing, as the result is not used. Is it needed?
598+
_ <- findAnnotation(d.mods, "http").collect({ case Apply(_, args) => args }).toList
570599
params <- d.vparamss
571600
_ = require(params.length == 1, s"RPC call ${d.name} has more than one request parameter")
572601
p <- params.headOption.toList

‎modules/internal/src/main/scala/higherkindness/mu/rpc/internal/util/FileUtil.scala

+1-1
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@ object FileUtil {
3232
def allFiles: Seq[File] = allMatching(_ => true)
3333

3434
def allMatching(f: File => Boolean): Seq[File] =
35-
if (file.isDirectory) file.listFiles.flatMap(_.allMatching(f))
35+
if (file.isDirectory) file.listFiles.toIndexedSeq.flatMap(_.allMatching(f))
3636
else Seq(file).filter(f)
3737

3838
def write(lines: Seq[String]): Unit = {

‎modules/internal/src/main/scala/higherkindness/mu/rpc/internal/util/StringUtil.scala

-28
This file was deleted.

‎modules/kafka/src/main/scala/higherkindness/mu/rpc/kafka/KafkaManagementImpl.scala

+1-1
Original file line numberDiff line numberDiff line change
@@ -82,7 +82,7 @@ class KafkaManagementImpl[F[_]: ContextShift: Concurrent] private[kafka] (
8282
override def describeTopics(dtr: DescribeTopicsRequest): F[Topics] =
8383
for {
8484
kTopics <- adminClient.describeTopics(dtr.names)
85-
topics = kTopics.map { case (topic, desc) => TopicDescription.fromJava(desc) }.toList
85+
topics = kTopics.map { case (_, desc) => TopicDescription.fromJava(desc) }.toList
8686
} yield Topics(topics)
8787

8888
override def listConsumerGroupOffsets(

‎modules/metrics/dropwizard/src/test/scala/higherkindness/mu/rpc/dropwizard/DropwizardMetricsTests.scala

+1-8
Original file line numberDiff line numberDiff line change
@@ -36,14 +36,12 @@ object DropwizardMetricsTests extends Properties("DropWizardMetrics") {
3636
val classifier = "classifier"
3737

3838
def performAndCheckMetrics(
39-
methodInfo: GrpcMethodInfo,
4039
numberOfCalls: Int,
4140
expectedCount: Int,
4241
registry: MetricRegistry,
4342
gaugeName: String,
4443
gaugeType: GaugeType,
45-
op: IO[Unit],
46-
status: Option[Status] = None
44+
op: IO[Unit]
4745
): IO[Boolean] =
4846
(1 to numberOfCalls).toList
4947
.map(_ => op)
@@ -77,7 +75,6 @@ object DropwizardMetricsTests extends Properties("DropWizardMetrics") {
7775

7876
(for {
7977
op1 <- performAndCheckMetrics(
80-
methodInfo,
8178
numberOfCalls,
8279
numberOfCalls,
8380
registry,
@@ -86,7 +83,6 @@ object DropwizardMetricsTests extends Properties("DropWizardMetrics") {
8683
metrics.increaseActiveCalls(methodInfo, Some(classifier))
8784
)
8885
op2 <- performAndCheckMetrics(
89-
methodInfo,
9086
numberOfCalls,
9187
expectedCount = 0,
9288
registry,
@@ -106,7 +102,6 @@ object DropwizardMetricsTests extends Properties("DropWizardMetrics") {
106102
s"$prefix.$classifier.${methodInfo.serviceName}.${methodInfo.methodName}.messages.sent"
107103

108104
performAndCheckMetrics(
109-
methodInfo,
110105
numberOfCalls,
111106
numberOfCalls,
112107
registry,
@@ -125,7 +120,6 @@ object DropwizardMetricsTests extends Properties("DropWizardMetrics") {
125120
s"$prefix.$classifier.${methodInfo.serviceName}.${methodInfo.methodName}.messages.received"
126121

127122
performAndCheckMetrics(
128-
methodInfo,
129123
numberOfCalls,
130124
numberOfCalls,
131125
registry,
@@ -143,7 +137,6 @@ object DropwizardMetricsTests extends Properties("DropWizardMetrics") {
143137
val headersName = s"$prefix.$classifier.calls.header"
144138

145139
performAndCheckMetrics(
146-
methodInfo,
147140
numberOfCalls,
148141
numberOfCalls,
149142
registry,

‎modules/metrics/prometheus/src/test/scala/higherkindness/mu/rpc/prometheus/PrometheusMetricsTests.scala

+5-38
Original file line numberDiff line numberDiff line change
@@ -19,8 +19,6 @@ package higherkindness.mu.rpc.prometheus
1919
import cats.effect.IO
2020
import cats.implicits._
2121
import higherkindness.mu.rpc.internal.interceptors.GrpcMethodInfo
22-
import higherkindness.mu.rpc.internal.metrics.MetricsOps
23-
import higherkindness.mu.rpc.internal.metrics.MetricsOps._
2422
import higherkindness.mu.rpc.internal.metrics.MetricsOpsGenerators.{methodInfoGen, statusGen}
2523
import io.grpc.Status
2624
import io.prometheus.client.Collector.MetricFamilySamples
@@ -36,19 +34,15 @@ class PrometheusMetricsTests extends Properties("PrometheusMetrics") {
3634
val classifier = "classifier"
3735

3836
def performAndCheckMetrics(
39-
methodInfo: GrpcMethodInfo,
4037
numberOfCalls: Int,
4138
registry: CollectorRegistry,
4239
metricName: String,
43-
labelNames: List[String],
44-
labelValues: List[String],
45-
op: IO[Unit],
46-
status: Option[Status] = None
40+
op: IO[Unit]
4741
)(checkSamples: List[MetricFamilySamples.Sample] => Boolean): IO[Boolean] =
4842
(1 to numberOfCalls).toList
4943
.map(_ => op)
5044
.sequence_
51-
.map(_ => checkMetrics(registry, metricName, labelNames, labelValues)(checkSamples))
45+
.map(_ => checkMetrics(registry, metricName)(checkSamples))
5246

5347
def findRecordedMetric(
5448
metricName: String,
@@ -66,9 +60,7 @@ class PrometheusMetricsTests extends Properties("PrometheusMetrics") {
6660

6761
def checkMetrics(
6862
registry: CollectorRegistry,
69-
metricName: String,
70-
labelNames: List[String],
71-
labelValues: List[String]
63+
metricName: String
7264
)(checkSamples: List[MetricFamilySamples.Sample] => Boolean): Boolean =
7365
checkSamples(findRecordedMetricOrThrow(metricName, registry).samples.asScala.toList)
7466

@@ -100,21 +92,15 @@ class PrometheusMetricsTests extends Properties("PrometheusMetrics") {
10092
(for {
10193
metrics <- PrometheusMetrics.build[IO](registry, prefix)
10294
op1 <- performAndCheckMetrics(
103-
methodInfo,
10495
numberOfCalls,
10596
registry,
10697
metricName,
107-
List("classifier"),
108-
List(classifier),
10998
metrics.increaseActiveCalls(methodInfo, Some(classifier))
11099
)(checkSingleSamples(metricName, numberOfCalls.toDouble))
111100
op2 <- performAndCheckMetrics(
112-
methodInfo,
113101
numberOfCalls,
114102
registry,
115103
metricName,
116-
List("classifier"),
117-
List(classifier),
118104
metrics.decreaseActiveCalls(methodInfo, Some(classifier))
119105
)(checkSingleSamples(metricName, 0L))
120106
} yield op1 && op2).unsafeRunSync()
@@ -129,12 +115,9 @@ class PrometheusMetricsTests extends Properties("PrometheusMetrics") {
129115
(for {
130116
metrics <- PrometheusMetrics.build[IO](registry, prefix)
131117
op1 <- performAndCheckMetrics(
132-
methodInfo,
133118
numberOfCalls,
134119
registry,
135120
metricName,
136-
List("classifier", "service", "method"),
137-
List(classifier, methodInfo.serviceName, methodInfo.methodName),
138121
metrics.recordMessageReceived(methodInfo, Some(classifier))
139122
)(checkSingleSamples(metricName, numberOfCalls.toDouble))
140123
} yield op1).unsafeRunSync()
@@ -149,12 +132,9 @@ class PrometheusMetricsTests extends Properties("PrometheusMetrics") {
149132
(for {
150133
metrics <- PrometheusMetrics.build[IO](registry, prefix)
151134
op1 <- performAndCheckMetrics(
152-
methodInfo,
153135
numberOfCalls,
154136
registry,
155137
metricName,
156-
List("classifier"),
157-
List(classifier),
158138
metrics.recordHeadersTime(methodInfo, elapsed.toLong, Some(classifier))
159139
)(checkSeriesSamples(metricName, numberOfCalls, elapsed))
160140
} yield op1).unsafeRunSync()
@@ -175,13 +155,7 @@ class PrometheusMetricsTests extends Properties("PrometheusMetrics") {
175155
.map { _ =>
176156
checkMetrics(
177157
registry,
178-
metricName,
179-
List("classifier", "method", "status"),
180-
List(
181-
classifier,
182-
methodTypeDescription(methodInfo),
183-
statusDescription(MetricsOps.grpcStatusFromRawStatus(status))
184-
)
158+
metricName
185159
)(checkSeriesSamples(metricName, numberOfCalls, elapsed))
186160
}
187161
} yield op1).unsafeRunSync()
@@ -202,14 +176,7 @@ class PrometheusMetricsTests extends Properties("PrometheusMetrics") {
202176
.map { _ =>
203177
checkMetrics(
204178
registry,
205-
metricName,
206-
List("classifier", "service", "method", "status"),
207-
List(
208-
classifier,
209-
methodInfo.serviceName,
210-
methodInfo.methodName,
211-
statusDescription(MetricsOps.grpcStatusFromRawStatus(status))
212-
)
179+
metricName
213180
)(checkSeriesSamples(metricName, numberOfCalls, elapsed))
214181
}
215182
} yield op1).unsafeRunSync()

‎modules/srcgen/core/src/main/scala/higherkindness/mu/rpc/srcgen/Model.scala

+4-1
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,6 @@
1616

1717
package higherkindness.mu.rpc.srcgen
1818

19-
import higherkindness.mu.rpc.internal.util.StringUtil._
2019
import higherkindness.mu.rpc.protocol.{SerializationType => SerType, _}
2120
import shapeless.tag
2221
import shapeless.tag.@@
@@ -25,6 +24,10 @@ object Model {
2524

2625
import Toolbox.u._
2726

27+
private implicit class StringOps(val s: String) extends AnyVal {
28+
def trimAll: String = s.replaceAll("\\s", "")
29+
}
30+
2831
case class RpcDefinitions(
2932
outputName: String,
3033
outputPackage: Option[String],

‎modules/srcgen/core/src/main/scala/higherkindness/mu/rpc/srcgen/util/AstOptics.scala

+8-9
Original file line numberDiff line numberDiff line change
@@ -96,7 +96,7 @@ trait AstOptics extends Compat {
9696
val qualifier: Lens[Select, Tree] =
9797
Lens[Select, Tree](_.qualifier)(qualifier => select => Select(qualifier, select.name))
9898

99-
val newTpt: Lens[New, Tree] = Lens[New, Tree](_.tpt)(tpt => n => New(tpt))
99+
val newTpt: Lens[New, Tree] = Lens[New, Tree](_.tpt)(tpt => _ => New(tpt))
100100

101101
val name: Optional[Tree, String] =
102102
Optional[Tree, String] {
@@ -114,10 +114,10 @@ trait AstOptics extends Compat {
114114
val rpcTypeNameFromTypeConstructor: Optional[Tree, Tree] = Optional[Tree, Tree] {
115115
case ast._Ident(x) => Some(x)
116116
case ast._SingletonTypeTree(x) => Some(x)
117-
case ast._AppliedTypeTree(AppliedTypeTree(x, List(tpe))) => Some(tpe)
118-
case ast._AppliedTypeTree(AppliedTypeTree(x, List(_, tpe))) => Some(tpe)
117+
case ast._AppliedTypeTree(AppliedTypeTree(_, List(tpe))) => Some(tpe)
118+
case ast._AppliedTypeTree(AppliedTypeTree(_, List(_, tpe))) => Some(tpe)
119119
case _ => None
120-
}(name => identity)
120+
}(_ => identity)
121121

122122
val _StreamingConstructor: Prism[Ident, String] = Prism.partial[Ident, String] {
123123
case Ident(TypeName("Observable")) => "Observable"
@@ -195,7 +195,7 @@ trait AstOptics extends Compat {
195195
}
196196

197197
case _ => None
198-
}(ann => identity)
198+
}(_ => identity)
199199

200200
val annotations: Lens[Modifiers, List[Tree]] =
201201
Lens[Modifiers, List[Tree]](_.annotations)(anns =>
@@ -230,7 +230,7 @@ trait AstOptics extends Compat {
230230
Optional[Annotation, Annotation] {
231231
case x if x.name == name => Some(x)
232232
case _ => None
233-
}(n => identity)
233+
}(_ => identity)
234234

235235
sealed trait Annotation {
236236
def name: String
@@ -250,9 +250,8 @@ trait AstOptics extends Compat {
250250

251251
object Annotation {
252252

253-
val firstArg: Optional[Annotation, Tree] = Optional[Annotation, Tree](_.firstArg) { x =>
254-
identity
255-
}
253+
val firstArg: Optional[Annotation, Tree] =
254+
Optional[Annotation, Tree](_.firstArg)(_ => identity)
256255

257256
case class NoParamAnnotation(name: String) extends Annotation
258257
case class UnnamedArgsAnnotation(name: String, args: Seq[Tree]) extends Annotation

‎project/ProjectPlugin.scala

+4-1
Original file line numberDiff line numberDiff line change
@@ -104,7 +104,10 @@ object ProjectPlugin extends AutoPlugin {
104104
"com.beachape" %% "enumeratum" % V.enumeratum,
105105
"com.sksamuel.avro4s" %% "avro4s-core" % V.avro4s,
106106
"org.log4s" %% "log4s" % V.log4s
107-
)
107+
),
108+
// Disable this flag because quasiquotes trigger a lot of false positive warnings
109+
scalacOptions -= "-Wunused:patvars", // for Scala 2.13
110+
scalacOptions -= "-Ywarn-unused:params" // for Scala 2.12
108111
)
109112

110113
lazy val internalMonixSettings: Seq[Def.Setting[_]] = Seq(

0 commit comments

Comments
 (0)
Please sign in to comment.