Thread: [Codenarc-developer] ReturnAdder.java from Groovy codebase
Brought to you by:
chrismair
Menu
▾
▴
codenarc-developer
|
From: Hamlet D. <ham...@ca...> - 2010-11-23 11:35:56
|
Hi Chris, Just saw that you fixed this open issue "UnnecessaryIfStatement - misses if without explicit return". Did you take a look at ReturnAdder.java? I think we should just invoke this this utility method for all MethodNodes so that everyone can see the return statements it produces: http://svn.codehaus.org/groovy/trunk/groovy/groovy-core/src/main/org/codehaus/groovy/classgen/ReturnAdder.java -- Hamlet D'Arcy ham...@ca... Bugs item #3111181, was opened at 2010-11-17 21:26 Message generated for change (Comment added) made by chrismair You can respond by visiting: https://sourceforge.net/tracker/?func=detail&atid=1126573&aid=3111181&group_id=250145 Please note that this message will contain a full copy of the comment thread, including the initial issue submission, for this request, not just the latest update. Category: None Group: None >Status: Closed >Resolution: Fixed Priority: 5 Private: No Submitted By: Chris Mair (chrismair) Assigned to: Nobody/Anonymous (nobody) Summary: UnnecessaryIfStatement - misses if without explicit return Initial Comment: >From Venkat's "Improving Your Groovy Code Quality" presentation at Spring 2GX (http://agiledeveloper.com/downloads.html) - slide #20 UnnecessaryIfStatement - misses unnecessary if/else when explicit return is omitted. e.g. def isSpellingCorrect(word) { File file = new File("...") def found = false file.eachLine { if (it == word) found = true } if (found) true else false } ---------------------------------------------------------------------- >Comment By: Chris Mair (chrismair) Date: 2010-11-23 06:26 Message: Fixed. Just added check for any if/else such as: if (condition) true; false. Will be part of version 0.12. - Chris ---------------------------------------------------------------------- Comment By: Hamlet D'Arcy (hamletdrc) Date: 2010-11-18 06:28 Message: You can see the logic for return statements in the groovy codebase. search for ReturnAdder.java. We can analyze this file and then replicate it. (or maybe invoke it!) ---------------------------------------------------------------------- You can respond by visiting: https://sourceforge.net/tracker/?func=detail&atid=1126573&aid=3111181&group_id=250145 |
|
From: <chr...@wa...> - 2010-11-23 13:41:46
|
Hamlet, Yes, I did take a look at ReturnAdder (thanks for that link). I sent you an email shortly after closing that issue.The way I see it, any if statement of the form if (condition) true; else false (or false/true) is unnecessary. It can either be simplified to a single expression or else removed altogether. So, I was able to fix this issue in a pretty straightforward way. I am also planning on expanding the rule to also catch statements of the form if (condition) <constant> [else <constant>] as long as they are not the last statement in a block. So, although that ReturnAdder code is interesting, I don't yet see the need for it. Thanks. Chris Hamlet DArcy <ham...@ca...> 11/23/2010 06:35 AM To codenarc-developer <cod...@li...> cc Subject [Codenarc-developer] ReturnAdder.java from Groovy codebase Hi Chris, Just saw that you fixed this open issue "UnnecessaryIfStatement - misses if without explicit return". Did you take a look at ReturnAdder.java? I think we should just invoke this this utility method for all MethodNodes so that everyone can see the return statements it produces: http://svn.codehaus.org/groovy/trunk/groovy/groovy-core/src/main/org/codehaus/groovy/classgen/ReturnAdder.java -- Hamlet D'Arcy ham...@ca... Bugs item #3111181, was opened at 2010-11-17 21:26 Message generated for change (Comment added) made by chrismair You can respond by visiting: https://sourceforge.net/tracker/?func=detail&atid=1126573&aid=3111181&group_id=250145 Please note that this message will contain a full copy of the comment thread, including the initial issue submission, for this request, not just the latest update. Category: None Group: None >Status: Closed >Resolution: Fixed Priority: 5 Private: No Submitted By: Chris Mair (chrismair) Assigned to: Nobody/Anonymous (nobody) Summary: UnnecessaryIfStatement - misses if without explicit return Initial Comment: >From Venkat's "Improving Your Groovy Code Quality" presentation at Spring 2GX (http://agiledeveloper.com/downloads.html) - slide #20 UnnecessaryIfStatement - misses unnecessary if/else when explicit return is omitted. e.g. def isSpellingCorrect(word) { File file = new File("...") def found = false file.eachLine { if (it == word) found = true } if (found) true else false } ---------------------------------------------------------------------- >Comment By: Chris Mair (chrismair) Date: 2010-11-23 06:26 Message: Fixed. Just added check for any if/else such as: if (condition) true; false. Will be part of version 0.12. - Chris ---------------------------------------------------------------------- Comment By: Hamlet D'Arcy (hamletdrc) Date: 2010-11-18 06:28 Message: You can see the logic for return statements in the groovy codebase. search for ReturnAdder.java. We can analyze this file and then replicate it. (or maybe invoke it!) ---------------------------------------------------------------------- You can respond by visiting: https://sourceforge.net/tracker/?func=detail&atid=1126573&aid=3111181&group_id=250145 ------------------------------------------------------------------------------ Increase Visibility of Your 3D Game App & Earn a Chance To Win $500! Tap into the largest installed PC base & get more eyes on your game by optimizing for Intel(R) Graphics Technology. Get started today with the Intel(R) Software Partner Program. Five $500 cash prizes are up for grabs. http://p.sf.net/sfu/intelisp-dev2dev _______________________________________________ Codenarc-developer mailing list Cod...@li... https://lists.sourceforge.net/lists/listinfo/codenarc-developer ForwardSourceID:NT000CFEB2 |
|
From: Hamlet D. <ham...@ca...> - 2010-11-23 13:46:10
|
The ReturnAdded needs to be invoked for "ArrayMethodReturnsNull", "BooleanMethodReturnsNull", and "CollectionMethodReturnsNull" rules. Currently, these rules only work if you use explicit return statements in your code, which most people don't. The ReturnAdder code seems quite general, and I think it would be alright to just invoke it for at least these three rules. We probably should not invoke it for every rule automatically because it does change the AST (perhaps the ExplicitReturnRule would break?) -- Hamlet D'Arcy ham...@ca... Hamlet, Yes, I did take a look at ReturnAdder (thanks for that link). I sent you an email shortly after closing that issue.The way I see it, any if statement of the form if (condition) true; else false (or false/true) is unnecessary. It can either be simplified to a single expression or else removed altogether. So, I was able to fix this issue in a pretty straightforward way. I am also planning on expanding the rule to also catch statements of the form if (condition) <constant> [else <constant>] as long as they are not the last statement in a block. So, although that ReturnAdder code is interesting, I don't yet see the need for it. Thanks. Chris Hamlet DArcy <ham...@ca...> 11/23/2010 06:35 AM To codenarc-developer <cod...@li...> cc Subject [Codenarc-developer] ReturnAdder.java from Groovy codebase Hi Chris, Just saw that you fixed this open issue "UnnecessaryIfStatement - misses if without explicit return". Did you take a look at ReturnAdder.java? I think we should just invoke this this utility method for all MethodNodes so that everyone can see the return statements it produces: http://svn.codehaus.org/groovy/trunk/groovy/groovy-core/src/main/org/codehaus/groovy/classgen/ReturnAdder.java -- Hamlet D'Arcy ham...@ca... Bugs item #3111181, was opened at 2010-11-17 21:26 Message generated for change (Comment added) made by chrismair You can respond by visiting: https://sourceforge.net/tracker/?func=detail&atid=1126573&aid=3111181&group_id=250145 Please note that this message will contain a full copy of the comment thread, including the initial issue submission, for this request, not just the latest update. Category: None Group: None >Status: Closed >Resolution: Fixed Priority: 5 Private: No Submitted By: Chris Mair (chrismair) Assigned to: Nobody/Anonymous (nobody) Summary: UnnecessaryIfStatement - misses if without explicit return Initial Comment: >From Venkat's "Improving Your Groovy Code Quality" presentation at Spring 2GX (http://agiledeveloper.com/downloads.html) - slide #20 UnnecessaryIfStatement - misses unnecessary if/else when explicit return is omitted. e.g. def isSpellingCorrect(word) { File file = new File("...") def found = false file.eachLine { if (it == word) found = true } if (found) true else false } ---------------------------------------------------------------------- >Comment By: Chris Mair (chrismair) Date: 2010-11-23 06:26 Message: Fixed. Just added check for any if/else such as: if (condition) true; false. Will be part of version 0.12. - Chris ---------------------------------------------------------------------- Comment By: Hamlet D'Arcy (hamletdrc) Date: 2010-11-18 06:28 Message: You can see the logic for return statements in the groovy codebase. search for ReturnAdder.java. We can analyze this file and then replicate it. (or maybe invoke it!) ---------------------------------------------------------------------- You can respond by visiting: https://sourceforge.net/tracker/?func=detail&atid=1126573&aid=3111181&group_id=250145 ------------------------------------------------------------------------------ Increase Visibility of Your 3D Game App & Earn a Chance To Win $500! Tap into the largest installed PC base & get more eyes on your game by optimizing for Intel(R) Graphics Technology. Get started today with the Intel(R) Software Partner Program. Five $500 cash prizes are up for grabs. http://p.sf.net/sfu/intelisp-dev2dev_______________________________________________ Codenarc-developer mailing list Cod...@li... https://lists.sourceforge.net/lists/listinfo/codenarc-developer ForwardSourceID:NT000CFEB2 ------------------------------------------------------------------------------ Increase Visibility of Your 3D Game App & Earn a Chance To Win $500! Tap into the largest installed PC base & get more eyes on your game by optimizing for Intel(R) Graphics Technology. Get started today with the Intel(R) Software Partner Program. Five $500 cash prizes are up for grabs. http://p.sf.net/sfu/intelisp-dev2dev _______________________________________________ Codenarc-developer mailing list Cod...@li... https://lists.sourceforge.net/lists/listinfo/codenarc-developer |
×
Want the latest updates on software, tech news, and AI?
Get latest updates about software, tech news, and AI from SourceForge directly in your inbox once a month.
Thanks for helping keep SourceForge clean.
X