[
https://issues.apache.org/jira/browse/OFBIZ-9702?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Julian Leichert updated OFBIZ-9702:
-----------------------------------
Attachment: OFBIZ-9702_org.apache.ofbiz.widget.renderer.macro_bugfixes.patch
class MacroFormRenderer
- added StringBuffer to create String extraParameter and ajaxParams
- removed redundant null-checks
- removed modelFormField.getForm() in line 1062,1000 because it does not do
anything
- Line 1639: changed Integer.toString to String.valueOf for better null
handling
class MacroScreenRenderer
- Fixed minor Findbugs Warnings
- Line 1022: added null-check
> [FB] Package org.apache.ofbiz.widget.renderer.macro
> ---------------------------------------------------
>
> Key: OFBIZ-9702
> URL: https://issues.apache.org/jira/browse/OFBIZ-9702
> Project: OFBiz
> Issue Type: Sub-task
> Components: ALL APPLICATIONS, ALL COMPONENTS
> Affects Versions: Trunk
> Reporter: Julian Leichert
> Priority: Minor
> Attachments:
> OFBIZ-9702_org.apache.ofbiz.widget.renderer.macro_bugfixes.patch
>
>
> MacroFormRenderer.java:237, RCN_REDUNDANT_NULLCHECK_OF_NONNULL_VALUE
> - RCN: Redundant nullcheck of fieldMap, which is known to be non-null in
> org.apache.ofbiz.widget.renderer.macro.MacroFormRenderer.renderDisplayField(Appendable,
> Map, ModelFormField$DisplayField)
> This method contains a redundant check of a known non-null value against the
> constant null.
> MacroFormRenderer.java:246, SBSC_USE_STRINGBUFFER_CONCATENATION
> - SBSC:
> org.apache.ofbiz.widget.renderer.macro.MacroFormRenderer.renderDisplayField(Appendable,
> Map, ModelFormField$DisplayField) concatenates strings using + in a loop
> The method seems to be building a String using concatenation in a loop. In
> each iteration, the String is converted to a StringBuffer/StringBuilder,
> appended to, and converted back to a String. This can lead to a cost
> quadratic in the number of iterations, as the growing string is recopied in
> each iteration.
> Better performance can be obtained by using a StringBuffer (or StringBuilder
> in Java 1.5) explicitly.
> For example:
> // This is bad
> String s = "";
> for (int i = 0; i < field.length; ++i) {
> s = s + field[i];
> }
> // This is better
> StringBuffer buf = new StringBuffer();
> for (int i = 0; i < field.length; ++i) {
> buf.append(field[i]);
> }
> String s = buf.toString();
> MacroFormRenderer.java:538, DM_BOXED_PRIMITIVE_FOR_PARSING
> - Bx: Boxing/unboxing to parse a primitive
> org.apache.ofbiz.widget.renderer.macro.MacroFormRenderer.renderDateTimeField(Appendable,
> Map, ModelFormField$DateTimeField)
> A boxed primitive is created from a String, just to extract the unboxed
> primitive value. It is more efficient to just call the static parseXXX method.
> MacroFormRenderer.java:1000, RV_RETURN_VALUE_IGNORED_NO_SIDE_EFFECT
> Return value of method without side effect is ignored
> This code calls a method and ignores the return value. However our analysis
> shows that the method (including its implementations in subclasses if any)
> does not produce any effect other than return value. Thus this call can be
> removed.
> We are trying to reduce the false positives as much as possible, but in some
> cases this warning might be wrong. Common false-positive cases include:
> - The method is designed to be overridden and produce a side effect in other
> projects which are out of the scope of the analysis.
> - The method is called to trigger the class loading which may have a side
> effect.
> - The method is called just to get some exception.
> If you feel that our assumption is incorrect, you can use a @CheckReturnValue
> annotation to instruct FindBugs that ignoring the return value of this method
> is acceptable.
> MacroFormRenderer.java:1062, RV_RETURN_VALUE_IGNORED_NO_SIDE_EFFECT,
> Priorität: Normal
> Return value of method without side effect is ignored
> This code calls a method and ignores the return value. However our analysis
> shows that the method (including its implementations in subclasses if any)
> does not produce any effect other than return value. Thus this call can be
> removed.
> We are trying to reduce the false positives as much as possible, but in some
> cases this warning might be wrong. Common false-positive cases include:
> - The method is designed to be overridden and produce a side effect in other
> projects which are out of the scope of the analysis.
> - The method is called to trigger the class loading which may have a side
> effect.
> - The method is called just to get some exception.
> If you feel that our assumption is incorrect, you can use a @CheckReturnValue
> annotation to instruct FindBugs that ignoring the return value of this method
> is acceptable.
> MacroFormRenderer.java:1275, DM_CONVERT_CASE, Priorität: Niedrig
> Dm: Use of non-localized String.toUpperCase() or String.toLowerCase() in
> org.apache.ofbiz.widget.renderer.macro.MacroFormRenderer.renderFieldTitle(Appendable,
> Map, ModelFormField)
> A String is being converted to upper or lowercase, using the platform's
> default encoding. This may result in improper conversions when used with
> international characters. Use the
> String.toUpperCase( Locale l )
> String.toLowerCase( Locale l )
> versions instead.
> MacroFormRenderer.java:1639, NP_NULL_ON_SOME_PATH
> - NP: Possible null pointer dereference of itemIndex in
> org.apache.ofbiz.widget.renderer.macro.MacroFormRenderer.renderFormatItemRowOpen(Appendable,
> Map, ModelForm)
> There is a branch of statement that, if executed, guarantees that a null
> value will be dereferenced, which would generate a NullPointerException when
> the code is executed. Of course, the problem might be that the branch or
> statement is infeasible and that the null pointer exception can't ever be
> executed; deciding that is beyond the ability of FindBugs.
> MacroFormRenderer.java:2339, RCN_REDUNDANT_NULLCHECK_OF_NONNULL_VALUE
> - RCN: Redundant nullcheck of prepLinkText, which is known to be non-null in
> org.apache.ofbiz.widget.renderer.macro.MacroFormRenderer.renderNextPrev(Appendable,
> Map, ModelForm)
> This method contains a redundant check of a known non-null value against the
> constant null.
> MacroFormRenderer.java:2979, SBSC_USE_STRINGBUFFER_CONCATENATION
> - SBSC:
> org.apache.ofbiz.widget.renderer.macro.MacroFormRenderer.createAjaxParamsFromUpdateAreas(List,
> String, Map) concatenates strings using + in a loop
> The method seems to be building a String using concatenation in a loop. In
> each iteration, the String is converted to a StringBuffer/StringBuilder,
> appended to, and converted back to a String. This can lead to a cost
> quadratic in the number of iterations, as the growing string is recopied in
> each iteration.
> Better performance can be obtained by using a StringBuffer (or StringBuilder
> in Java 1.5) explicitly.
> For example:
> // This is bad
> String s = "";
> for (int i = 0; i < field.length; ++i) {
> s = s + field[i];
> }
> // This is better
> StringBuffer buf = new StringBuffer();
> for (int i = 0; i < field.length; ++i) {
> buf.append(field[i]);
> }
> String s = buf.toString();
> MacroFormRenderer.java:3069, RCN_REDUNDANT_NULLCHECK_WOULD_HAVE_BEEN_A_NPE
> - RCN: Nullcheck of modelFormField at line 3083 of value previously
> dereferenced in
> org.apache.ofbiz.widget.renderer.macro.MacroFormRenderer.makeHyperlinkByType(Appendable,
> String, String, String, String, Map, String, String, String, ModelFormField,
> HttpServletRequest, HttpServletResponse, Map)
> A value is checked here to see whether it is null, but this value can't be
> null because it was previously dereferenced and if it were null a null
> pointer exception would have occurred at the earlier dereference.
> Essentially, this code and the previous dereference disagree as to whether
> this value is allowed to be null. Either the check is redundant or the
> previous dereference is erroneous.
> MacroFormRenderer.java:3188, RCN_REDUNDANT_NULLCHECK_OF_NONNULL_VALUE
> RCN: Redundant nullcheck of hiddenFormName, which is known to be non-null in
> org.apache.ofbiz.widget.renderer.macro.MacroFormRenderer.makeHyperlinkString(Appendable,
> String, String, String, Map, String, String, ModelFormField,
> HttpServletRequest, HttpServletResponse, Map, String)
> This method contains a redundant check of a known non-null value against the
> constant null.
> MacroFormRenderer.java:3240, RCN_REDUNDANT_NULLCHECK_OF_NONNULL_VALUE
> - RCN: Redundant nullcheck of hiddenFormName, which is known to be non-null
> in
> org.apache.ofbiz.widget.renderer.macro.MacroFormRenderer.makeHiddenFormLinkAnchor(Appendable,
> String, String, String, ModelFormField, HttpServletRequest,
> HttpServletResponse, Map)
> This method contains a redundant check of a known non-null value against the
> constant null.
> MacroScreenRenderer.java:443, DM_CONVERT_CASE
> - Dm: Use of non-localized String.toUpperCase() or String.toLowerCase() in
> org.apache.ofbiz.widget.renderer.macro.MacroScreenRenderer.renderContentEnd(Appendable,
> Map, ModelScreenWidget$Content)
> A String is being converted to upper or lowercase, using the platform's
> default encoding. This may result in improper conversions when used with
> international characters. Use the
> String.toUpperCase( Locale l )
> String.toLowerCase( Locale l )
> versions instead.
> MacroScreenRenderer.java:443, RV_CHECK_FOR_POSITIVE_INDEXOF
> - RV:
> org.apache.ofbiz.widget.renderer.macro.MacroScreenRenderer.renderContentEnd(Appendable,
> Map, ModelScreenWidget$Content) checks to see if result of String.indexOf is
> positive
> The method invokes String.indexOf and checks to see if the result is positive
> or non-positive. It is much more typical to check to see if the result is
> negative or non-negative. It is positive only if the substring checked for
> occurs at some place other than at the beginning of the String.
> MacroScreenRenderer.java:555, DM_CONVERT_CASE
> - Dm: Use of non-localized String.toUpperCase() or String.toLowerCase() in
> org.apache.ofbiz.widget.renderer.macro.MacroScreenRenderer.renderSubContentEnd(Appendable,
> Map, ModelScreenWidget$SubContent)
> A String is being converted to upper or lowercase, using the platform's
> default encoding. This may result in improper conversions when used with
> international characters. Use the
> String.toUpperCase( Locale l )
> String.toLowerCase( Locale l )
> versions instead.
> MacroScreenRenderer.java:555, RV_CHECK_FOR_POSITIVE_INDEXOF
> - RV:
> org.apache.ofbiz.widget.renderer.macro.MacroScreenRenderer.renderSubContentEnd(Appendable,
> Map, ModelScreenWidget$SubContent) checks to see if result of String.indexOf
> is positive
> The method invokes String.indexOf and checks to see if the result is positive
> or non-positive. It is much more typical to check to see if the result is
> negative or non-negative. It is positive only if the substring checked for
> occurs at some place other than at the beginning of the String.
> MacroScreenRenderer.java:726, DM_CONVERT_CASE
> - Dm: Use of non-localized String.toUpperCase() or String.toLowerCase() in
> org.apache.ofbiz.widget.renderer.macro.MacroScreenRenderer.renderScreenletPaginateMenu(Appendable,
> Map, ModelScreenWidget$Form)
> A String is being converted to upper or lowercase, using the platform's
> default encoding. This may result in improper conversions when used with
> international characters. Use the
> String.toUpperCase( Locale l )
> String.toLowerCase( Locale l )
> versions instead.
> MacroScreenRenderer.java:1022, NP_NULL_ON_SOME_PATH
> - NP: Possible null pointer dereference of modelScreen in
> org.apache.ofbiz.widget.renderer.macro.MacroScreenRenderer.renderPortalPagePortletBody(Appendable,
> Map, ModelScreenWidget$PortalPage, GenericValue)
> There is a branch of statement that, if executed, guarantees that a null
> value will be dereferenced, which would generate a NullPointerException when
> the code is executed. Of course, the problem might be that the branch or
> statement is infeasible and that the null pointer exception can't ever be
> executed; deciding that is beyond the ability of FindBugs.
--
This message was sent by Atlassian JIRA
(v6.4.14#64029)
