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.

Sign up for GitHub

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

Jump to bottom

Rewrite rule for Map#mapValues should call .view #216

Open
NthPortal opened this issue Jun 10, 2019 · 4 comments
Open

Rewrite rule for Map#mapValues should call .view #216

NthPortal opened this issue Jun 10, 2019 · 4 comments
Labels
bug Something isn't working good first issue Good for newcomers rewrite Related to rewrite rules (not compat library code)

Comments

@NthPortal
Copy link
Contributor

Rewrite rule for Map#mapValues should call .view before calling .mapValues (H/T @joshlemer)
@ekrich
Copy link
Contributor

ekrich commented Jun 24, 2019

This involves the same code but requesting that we have a way to update the code to remove the deprecation and still compile against 2.11 and 2.12.

[warn] /Users/eric/workspace/sconfig/sconfig/jvm/src/test/scala/org/ekrich/config/impl/PublicApiTest.scala:170:8: method mapValues in trait MapOps is deprecated (since 2.13.0): Use .view.mapValues(f). A future version will include a strict version of this method (for now, .view.mapValues(f).toMap).
[warn]       .mapValues(intValue(_): AbstractConfigValue)

The code in question:

val aMapValue = Map("a" -> 1, "b" -> 2, "c" -> 3)
      .mapValues(intValue(_): AbstractConfigValue)
      .toMap

@joshlemer
Copy link

It's also probably worth pointing out that .transform((_, v) => f(v)) is going to be a lot more efficient than .view.mapValues(f).toMap

@NthPortal
Copy link
Contributor Author

@joshlemer yes, but that's a less straightforward rewrite, due to having to rewrite the function too

@ekrich
Copy link
Contributor

ekrich commented Jun 24, 2019

I only had that in the code base 3 places so it certainly is an easy workaround for me.

  • Eliminated deprecation warning on 2.13
  • Works on 2.11, 2.12, and 2.13

I would never thought of it though.

@NthPortal NthPortal added bug Something isn't working rewrite Related to rewrite rules (not compat library code) labels May 1, 2020
2m added a commit to 2m/tapir that referenced this issue Aug 26, 2020
@2m
Use transform when mapping Map values
e7272f0 
This fixes the following warning generated when tapir is being
used with Scala 2.13:

```
method mapValues in trait MapOps is deprecated (since 2.13.0): Use .view.mapValues(f). A future version will include a strict version of this method (for now, .view.mapValues(f).toMap)
```

I am using tapir in my project with warnings-as-errors enabled. This
warning prevents me from using `.in(formBody[...])` with case
classes.

The idea to switch to `transform` which is compatible with Scala 2.11,
2.12 and 2.13 was originally found here: scala/scala-collection-compat#216 (comment)
@2m 2m mentioned this issue Aug 26, 2020
Merged
@SethTisue SethTisue added the good first issue Good for newcomers label Oct 28, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug Something isn't working good first issue Good for newcomers rewrite Related to rewrite rules (not compat library code)
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
4 participants
@SethTisue @ekrich @joshlemer @NthPortal