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

Support ByteStreams.toByteArray -> inputStream.readAllBytes() #390

Open
koppor opened this issue Jan 12, 2024 · 8 comments
Open

Support ByteStreams.toByteArray -> inputStream.readAllBytes() #390

koppor opened this issue Jan 12, 2024 · 8 comments
Labels
good first issue Good for newcomers recipe Recipe requested

Comments

@koppor
Copy link

koppor commented Jan 12, 2024

Instead of Guava's ByteStreams#toByteArray(), InputStream.html#readAllBytes should be used. The latter is available since Java 9.

- String inputStr = new String(ByteStreams. toByteArray(inputStream), Charsets.UJTF_8);
+ String inputStr = new String(inputStream.readAllBytes()), Charsets.UJTF_8);

Should IMHO be doable with Refaster templates?

@timtebeek timtebeek transferred this issue from openrewrite/rewrite Jan 12, 2024
@timtebeek
Copy link
Contributor

Indeed looks like a good candidate! I've moved the issue to rewrite-migrate-java since that's where we already have our other Guava recipes and the rewrite-templating dependency. I'm doubting if the recipe would even need the new String(... ) surrounding it. (anyone) welcome to give this a go! :)

@Stephan202
Copy link

I'm doubting if the recipe would even need the new String(... ) surrounding it.

Indeed, better to omit that and make the rule more generally applicable. 👍 I filed PicnicSupermarket/error-prone-support#963, but feel free to copy over to this repo. Sharing is caring ;)

@Siedlerchr
Copy link

Just be careful, this does not work under Android 9 & 10. I recently had the issue...

@timtebeek
Copy link
Contributor

I'm doubting if the recipe would even need the new String(... ) surrounding it.

Indeed, better to omit that and make the rule more generally applicable. 👍 I filed PicnicSupermarket/error-prone-support#963, but feel free to copy over to this repo. Sharing is caring ;)

Probably good to have our own copy here as well, to fit in with our no guava recipes. Thanks for the reference implementation @Stephan202 ! Would you want to add that here @koppor ? Then we can also run it at scale here.

Longer term we could look at adding Picnicsupermarket/error-prone-support as a dependency here and reference recipes from there in our composite yaml recipes, but that first needs a new release of EPS and some work on correctly attributing those recipes to the authors there. :)

@Stephan202
Copy link

but that first needs a new release of EPS

@rickie and I have started to talk about doing a new release. Could be a few weeks (lots of other stuff going on), but hopefully less!

@timtebeek
Copy link
Contributor

Turns out we have an interesting challenge when we want to integrate this into rewrite-migrate-java, which still uses Java 8.
image

We might need to add a second source set of Refaster recipe classes does no make it into the the final jar, but do contribute the generated Java 8 runtime compatible recipes. 🤔 Fun corner case this. /cc @knutwannheden

@knutwannheden
Copy link
Contributor

Yes, this issue has also crossed my mind. And indeed I think we should be able to add a separate source set. Another question would be if this should somehow automatically add a precondition for the required Java version. We should be able to check that somehow. But that would be more of a nice-to-have, I think.

@timtebeek
Copy link
Contributor

Neat; figured get start with that in #399; Although come to think of it that might require us to complete openrewrite/rewrite-templating#57 first

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Good for newcomers recipe Recipe requested
Projects
Status: Recipes Wanted
Development

Successfully merging a pull request may close this issue.

5 participants