JnRouvignac / AutoRefactor

Eclipse plugin to automatically refactor Java code bases
Other
176 stars 37 forks source link

JUnitAssertRefactoring - different opinion for "fail" refactorings #277

Open cal101 opened 7 years ago

cal101 commented 7 years ago

Hi @JnRouvignac @Fabrice-TIERCELIN,

using the experimental command line interface of autorefactor I am trying refactorings on some bigger source sets.

I understand that autorefactor is an opinionated sets of rules and I am agreeing with most of those opinions. But in case of JUnitAssertRefactoring I disagree with (some of) the refactorings of "fail".

For example look at the following diff:

-            if (null != results) {
-                fail(m + "query failed XPath: " + results +
-                        "\n xml response was: " + response +
-                        "\n request was: " + req.getParamString());
-            }
+            assertNull(m + "query failed XPath: " + results +
+                               "\n xml response was: " + response +
+                               "\n request was: " + req.getParamString(), results);

"result" and or "response" may be big strings or objects having an expensive toString or producing big strings. So this code is written this way to avoid those costs. Something along the logging idiom

if (log.isDebugEnabled()) {
    log.debug(<expensive-to-string>);
}

Would you consider some kind of configuration in cases like this? If you can tell me an example of how to configure a refactoring I may be able to develop something for this case.

Fabrice-TIERCELIN commented 7 years ago

You're right. Currently, you can disable one refactoring going to Window -> Preferences -> Java -> Code style -> AutoRefactor -> Rules by default. In the future, we should split the JUnit rule to allow to enable/disable only this part of the rule. We planned to do such split for the next version (not the next release) but we don't know when we will release.

cal101 commented 7 years ago

Do you have any prototype work or ideas how to do it? I think about giving names to the different parts and being able to enable or disable parts or configure them. Something like

--param JUnitAssertRefactoring.ifToFail=false

and the rule providing Parameter objects including descriptions. Something that allows ui usage and command line usage.