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

Remove warnings #1994

Open
wants to merge 57 commits into
base: main
Choose a base branch
from
Open

Remove warnings #1994

wants to merge 57 commits into from

Conversation

Scaalp-fr
Copy link

@Scaalp-fr Scaalp-fr commented Dec 11, 2021

Mailing List thread:

The purpose of this pull request is to remove as much warnings as possible. This pull request is maybe a bit huge.

A summary of the modifications made:

  • Replace using constructors of boxed java base types by Type.valueOf. E.g new java.lang.Long(l) => java.lang.Long.valueOf(l).
  • Use new versions of deprecated lift functions (except in mongodb_record).
  • Use Class.getDeclaredConstructor().newInstance() instead of deprecated function Class.newInstance.
  • Add empty argument list where needed. Procedure syntax is deprecated.
  • Add @unchecked annotation to non exhaustive pattern matchings.
  • Remove deprecated (since 3.3.0) object DontMergeAttributes. Use its replacement object DontMergeClass. This could be a breaking change, but these objects are probably only used internally.
  • Use RoundingMode.HALF_UP instead of deprecated BigDecimal.ROUND_HALF_UP.
  • Remove unused imports.
  • Use method foldLeft instead of deprecated "/:".
  • Replace method TestHandler.then by TestHandler.andThen. "then" is a reserved word (since scala 2.10.0); usage as an identifier is deprecated. This could be a breaking change.
  • Don't use deprecated implicit conversion from Long to TimeSpan. Make explicit conversion.
  • Delete empty files XmlMenu.scala and JSONComet.scala.
  • Remove some final modifiers, for some case classes. The compiler reports warning for these cases. To keep the final keyword, the classes should be declared in the companion object, but that would need the addition of a new import to the users, which would be a breaking change. This concerns the classes LiftScreen, SHtml and RestHelper.
  • Remove tests concerning deprecated trait NodeSeqFunc, so that the test don't show deprecated warnings. (should it be kept ?)
  • Replace deprecated view bounds by implicit arguments.
  • Replace deprecated Travesable by Iterable.
  • Use scala.jdk.CollectionConverters instead of scala.collection.JavaConverters. Add scala-compat-library to make it work in scala 2.11 and scala 2.12.
  • Conversion from Array to List (or Set) changed from List(a:_*) to a.toList.
  • Replace local variable names "enum" by "e", because enum will become a reserved keyword in scala 3.
  • Add backticks in class EnumWithDescription, instead of renaming it, to avoid a breaking change. This class should maybe be deprecated?
  • Remove dead code.
  • Replace "0l" by "0L".
  • Didn't care about use of deprecated warnings in mongodb_record (concerning code that should be replaced with BsonableField).
  • Replace deprecated syntax 'rec by Symbol("rec").
  • Scala 2.13 cannot compile record (method displayName in object RecordRules). I am not proficient enough in scala to know how to rewrite the code.
  • squeryl-record cannot be compiled in scala 2.13. Available squeryl librairies for scala 2.13 have dropped many methods used by the code.
  • Activated compiler options to report unused imports, private and local variables and methods and remove them. Some compiler flags have been added to replace some imports.

pascal added 30 commits December 4, 2021 17:11
pascal added 6 commits December 9, 2021 17:34
As compiler option is used for the same effect. The advantage is that
removing these import get rid of unused import messages.
Modify the code where it is possible.
Add the scalac option -no-link-warnings for the remaining warnings.
@Shadowfiend
Copy link
Member

Add default case in pattern matching, by throwing an IllegalArgumentException exception, to satisfy the compiler. This could be a breaking change? Better to add the @unchecked annotation?

The only breaking thing here is that it used to throw a scala.MatchError. Can we throw that explicitly instead of IllegalArgumentException, or is that error also deprecated?

@andreak
Copy link
Member

andreak commented Dec 11, 2021 via email

@Scaalp-fr
Copy link
Author

I didn't think of using MatchError. It is not deprecated and it could be used instead of IllegalArgumentException. I'll wait the decision between @unchecked and an exception, and change the code accordingly.

Replace the added default case in pattern matching throwing an
IllegalArgumentException by an unchecked annotation.

Let the runtime throw the standard MatchError as previously, if the case
ever happens.
@Scaalp-fr
Copy link
Author

I replaced throwing of IllegalArgumentException by the @unchecked annotation, for non exhaustive pattern matches. I updated the pull request comment. This lets the runtime throw the MatchError exception as previously.

@kenwenzel
Copy link

Do you have any updates on this PR? I'm looking forward to use Lift with Scala 3.

@Scaalp-fr
Copy link
Author

No, I didn't put more time in this PR. I am waiting a reaction from the Lift maintainers, which don't have time to work on this project anymore. My purpose was also to use Lift with Scala 3. I was hopping that this PR would be integrated in the master branch, as a first step.

By the way, I think that libraries compiled with Scala 2.13 can be used with Scala 3. So it depends on which modules of Lift you need. The basic modules compile fine in Scala 3. The squeryl modules cannot be compiled for Scala 2.13, it must first be adapted to a more recent version of squeryl. In the persistence module, I only use db (I was using Mapper but I removed it, using my own code).

@kenwenzel
Copy link

Currently we only need the core and webkit modules. Maybe it is also possible to create some kind of "lift-light" that does not include any of the database extensions.

@dpp
Copy link
Member

dpp commented Feb 28, 2022

I'll spend some time on this PR this week

@andreak
Copy link
Member

andreak commented Mar 1, 2022

@dpp That's excellent news! It'd be awesome to have Lift working with Scala-3.

@Scaalp-fr
Copy link
Author

If I can do more to advance this PR, don't hesitate to ask.

@kenwenzel
Copy link

@dpp @Shadowfiend Can this PR be merged?

@Scaalp-fr
Copy link
Author

I don't expect anymore that this PR will be included in the master branch of the main repository. On my fork, I created a new branch for-use-in-scala-3 where I compile the modules I need using Scala 2.13.9. I had to do some minor changes in the lift code. For example, existential types are deprecated in Scala 3 which could include a breaking change if you use the one lift function using it. I then use this version of lift in my project which I recently migrated to Scala 3.2. It works and took much less time than I expected.

@andreak
Copy link
Member

andreak commented Sep 28, 2022

It's a shame nobody has time for this, me included. Please don't give up on this:-)

I have some suggested changes to Lift in my local fork which I'd also like to have included.

It'd be really cool if we could team up and get Lift ready for use with Scala >= 3.2. My company would be willing to sponsor work on this.

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

Successfully merging this pull request may close these issues.

5 participants