Skip to content
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

Basic usages triggers compiler warnings when using -Xlint #320

Open
eejbyfeldt opened this issue Sep 15, 2023 · 2 comments
Open

Basic usages triggers compiler warnings when using -Xlint #320

eejbyfeldt opened this issue Sep 15, 2023 · 2 comments
Assignees
Labels

Comments

@eejbyfeldt
Copy link

Describe the bug
Usage of the library triggers compiler warnings when setting -Xlint.

To Reproduce
Compiling code like (based on readme examples)

package test

import json._

object Test {
  case class UserId(value: String) extends AnyVal
  implicit val userIdSchema: json.Schema[UserId] = Json.schema[UserId].toDefinition("userId")
}

with -Xlint and `-Wconf:any:warning-verbose" results in warnings like

[warn] /home/eejbyfeldt/Downloads/hYlrLUvZTGO09M01f2EN7A/src/main/scala/main.scala:7:63: Implicit resolves to enclosing value userIdSchema
[warn] Applicable -Wconf / @nowarn filters for this warning: msg=<part of the message>, cat=w-flag-self-implicit, site=test.Test.userIdSchema
[warn]   implicit val userIdSchema: json.Schema[UserId] = Json.schema[UserId].toDefinition("userId")
[warn]                                                               ^
[warn] one warning found

Expected behavior
Basic usage of library should compile without warnings.

Actual results
Compiler warnings.

Versions:

  • jsonschema version 0.7.11
  • scala version 2.13.12

Additional context
Add any other context about the problem here.

@andyglow
Copy link
Owner

@eejbyfeldt
Copy link
Author

I do not understand the relevancy of the links the first one is just saying that the flag is good for catching potentially problematic code. The second link seems to be about some other bug that is fixed later versions of scalac so it seems likely we are not hitting that bug?

So I had a look at the code to see what triggers it and found this:

// FIXME: this method is pretty ugly
// it is trying to handle situation like this
// {{{
// case class CC(a: String)
// implicit val aSchema: Schema[CC] = Json.schema[CC]
// }}}
// in this case `inferImplicitValue` returns a self-reference
// which turns out to be resolved as `null` eventually
//
// So this method take a line of a source code where our def-macro is used and
// see if it is assignment and it assigns to variable returned by `inferImplicitValue`
//
def isSelfRef(x: Tree): Boolean = x match {
case x @ Select(This(_), TermName(field)) =>
val pos = x.pos
pos.source match {
case NoSourceFile => false
case src =>
val end = src.lineToOffset(src.offsetToLine(pos.point))
var start = end - 2
while ({
!src.isLineBreak(start) &&
!src.isEndOfLine(start)
}) start = start - 1
val str = new String(src.content, start, pos.point - start)
if (str.contains(field) && str.contains("=") && str.contains("implicit")) true
else false
}
case _ => false
}
which is guess is the hack that tries to handle the the macro will resolve an implicit pointing to it self. So I guess as long as that code correct it guess the warning would be a false positive, the only problem is that code will not correctly identify self references under lots of circumstances.

Lets take for example this code:

package test

import json._

object TestNullAtRuntime {
  case class UserId(value: String) extends AnyVal
  implicit val userIdSchema: json.Schema[ // Some nice comment
   UserId
  ] = Json.schema[UserId].toDefinition("userId")

  def main(args: Array[String]): Unit = {
    println(userIdSchema)
  }
}

Trying to run this code will crash with:

Caused by: java.lang.NullPointerException
	at json.Schema$def.deepCopy$1(Schema.scala:553)
	at json.Schema$def.toDefinition(Schema.scala:556)
	at test.TestNullAtRuntime$.<clinit>(main.scala:9)
	... 18 more

so it failed to detect the self implicit and the compiler warning is not a false positive and actually found the bug.

Code like this

package test

import json._

object TestDoesNotCompile {
  case class UserId(value: String) extends AnyVal

  def main(args: Array[String]): Unit = {
    implicit val userIdSchema: json.Schema[UserId] = Json.schema[UserId].toDefinition("userId")
    println(userIdSchema)
  }
}

will fail compilation with

./src/main/scala/main_does_not_compile.scala:9:65: forward reference to value userIdSchema defined on line 9 extends over definition of value userIdSchema
     implicit val userIdSchema: json.Schema[UserId] = Json.schema[UserId].toDefinition("userId")

But my favorite one is code like this:

package test

import json._

object TestCLRF {
  case class UserId(value: String) extends AnyVal
  implicit val userIdSchema: json.Schema[UserId] =
    Json.schema[UserId].toDefinition("userId")
  def main(args: Array[String]): Unit = {
    println(userIdSchema)
  }
}

which compiles and does the correct thing if the file as LF line endings but change it to have CRLF and it fails at runtime with

Caused by: java.lang.NullPointerException
	at json.Schema$def.deepCopy$1(Schema.scala:553)
	at json.Schema$def.toDefinition(Schema.scala:556)
	at test.TestCLRF$.<clinit>(main_clrf.scala:8)
	... 18 more

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants