kevin-lee / refined4s

newtype and refinement (refined) type for Scala 3
https://refined4s.kevinly.dev
MIT License
10 stars 2 forks source link

Compatibility with JDK 21 - deprecated new URL #354

Closed ivan-klass closed 6 days ago

ivan-klass commented 2 months ago

Summary

[error]    |constructor URL in class URL is deprecated since : see corresponding Javadoc for more information.
[error] -- Error: modules/refined4s-core/shared/src/main/scala/refined4s/types/network.scala:82:12 
[error] 82 |        new URL(a)

[error] -- Error: modules/refined4s-core/shared/src/main/scala/refined4s/types/network.scala:98:27 
[error] 98 |      def toURL: URL = new URL(url.value)

[error] -- Error: modules/refined4s-core/shared/src/main/scala/refined4s/types/network.scala:183:14 
[error] 183 |          new java.net.URL(urlStr)

Version

latest

Description

Can't compile. OpenJDK 64-Bit Server VM Corretto-21.0.3.9.1 (build 21.0.3+9-LTS, mixed mode, sharing)

kevin-lee commented 2 months ago

It's not a problem with any projects using refined4s, but if it's an issue with building this library, could you please use a lower version of Java than 20? As you know, you can install multiple versions of Java and set a specific version for the project.

Regarding the issue and more about it, java.net.URL(String) constructors have been deprecated since Java 20. This library can't be compiled for 20 or higher as the users may use lower versions of Java. The suggested way from the deprecated comment from Java is using URI.toURL, but URL and URI are not exactly the same thing. So if anyone's already relying on the difference, fixing the constructor can introduce a bug. e.g.)

import java.net.{URI, URL}

new URL("http://blah.blah/foo bar")
// URL containing "http://blah.blah/foo bar"

new URI("http://blah.blah/foo bar")
// java.net.URISyntaxException: Illegal character in path at index 20: http://blah.blah/foo bar

For the users of refined4s with Java 20+, if they want to following that suggestion from JDK itself, they can do Uri(String).toUrl: Url or Uri(String).toURL: URL or they can still use Url(String) if that's what they want.

I added Uri.toUrl and Uri.toURL as well as Url.toUri and Url.toURI a few days ago, so they will be available in the next release.

ivan-klass commented 2 months ago

That's right. But if it's only the deprecation warning for the current version, it means it will be removed / runtime broken in upcoming JDK versions.

I would recommend remove java.net.URL from the core part into modules like refined4s-jdk. That module may later have separate versions for different target Java platform versions, if needed.

Also the code that imports java.net... types should definitely be in jvm/src, not shared/scr

kevin-lee commented 2 months ago

That's right. But if it's only the deprecation warning for the current version, it means it will be removed / runtime broken in upcoming JDK versions.

I would recommend remove java.net.URL from the core part into modules like refined4s-jdk. That module may later have separate versions for different target Java platform versions, if needed.

Also the code that imports java.net... types should definitely be in jvm/src, not shared/scr

@ivan-klass Oh, that's a good point.

Although I won't make a change to the deprecated constructor for now because its @Deprecated annotation still doesn't have forRemoval = true and I need to think more about the modularization of Url, I should address the java.net.URL in the Scala.js issue. It will be done in the next version.

Thank you!

kevin-lee commented 6 days ago

This issue should be addressed by v1.0.0 (#384).