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

Assignments inside condition do not work with Parenthesis removal #3913

Closed
koppor opened this issue Jan 14, 2024 · 5 comments
Closed

Assignments inside condition do not work with Parenthesis removal #3913

koppor opened this issue Jan 14, 2024 · 5 comments
Labels
bug Something isn't working

Comments

@koppor
Copy link

koppor commented Jan 14, 2024

I updated openrewrite:

-    id 'org.openrewrite.rewrite' version '6.6.3'
+    id 'org.openrewrite.rewrite' version '6.6.6'

-    rewrite(platform("org.openrewrite.recipe:rewrite-recipe-bom:2.5.4"))
+    rewrite(platform("org.openrewrite.recipe:rewrite-recipe-bom:2.6.1"))

Some more parentheses are removed here.

I have some WTF parts in the code with assignments inside (!) if conditions. When removing parentheses here, it leads to a compiler error

-        if ((str == null) || ((end = str.length()) == 0) || ((((ch = str.charAt(0)) < '0') || (ch > '9')) && (!(sign = ch == '-') || (++idx == end) || ((ch = str.charAt(idx)) < '0') || (ch > '9')))) {
+        if ((str == null) || ((end = str.length()) == 0) || ((((ch = str.charAt(0)) < '0') || (ch > '9')) && (!sign = ch == '-' || (++idx == end) || ((ch = str.charAt(idx)) < '0') || (ch > '9')))) {
C:\git-repositories\jabref-all\jabref\src\main\java\org\jabref\model\strings\StringUtil.java:486: error: unexpected type
        if ((str == null) || ((end = str.length()) == 0) || ((((ch = str.charAt(0)) < '0') || (ch > '9')) && (!sign = ch == '-' || (++idx == end) || ((ch = str.charAt(idx)) < '0') || (ch > '9')))) {
                                                                                                              ^
  required: variable
  found:    value
@timtebeek
Copy link
Contributor

Hi @koppor ; Thanks for the report! It seems we're not yet handling that !(sign = ch == '-') correctly in

public class UnnecessaryParenthesesVisitor<P> extends JavaVisitor<P> {

I'll move this issue and we can fix that in openrewrite/rewrite.

@timtebeek timtebeek transferred this issue from openrewrite/rewrite-static-analysis Jan 14, 2024
@koppor
Copy link
Author

koppor commented Jan 14, 2024

Thank you for moving around. I still have not (fully) learned in which repository which issue should go. I really praise GitHub that it implemented the move-around-feature!


The code in question came from http://stackoverflow.com/questions/1030479/most-efficient-way-of-converting-string-to-integer-in-java

@timtebeek
Copy link
Contributor

I understand it's sometimes not immediately clear where issues should be reported or fixed; You're right in reporting issues in the repository are located, and I'll move the issues where I think the fix might be elsewhere as it was here. Thanks again for the report; really helpful, and this case quickly fixed with 670dd5c

@koppor
Copy link
Author

koppor commented Jan 24, 2024

I can confirm that the fix works in the BOM 2.6.2 - JabRef/jabref#10813

@timtebeek
Copy link
Contributor

Glad to hear, thanks! Always nice to get that feedback. :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
Archived in project
Development

No branches or pull requests

2 participants