Re: [ofbiz-framework] branch trunk updated: Improved: Handle remaining checkstyle errors (OFBIZ-12169)

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
3 messages Options
Reply | Threaded
Open this post in threaded view
|

Re: [ofbiz-framework] branch trunk updated: Improved: Handle remaining checkstyle errors (OFBIZ-12169)

jleroux@apache.org
Hi Team,

I have been working on https://checkstyle.sourceforge.io/config_whitespace.html#MethodParamPad

I could fix few issues in code. But there are still some cases that can't be handled by changing code, notably when a casting is necessary, and not only.

I believe there is an error in checkstyle code. If you look at the URL above and the errors reported, you will see that the errors don't belong to the
tokens to check.

We can use this solution
https://stackoverflow.com/questions/45109089/checkstyle-can-i-change-the-should-be-on-the-previous-line-rule-of-meth#answer-45109414
But I believe we should rather discuss with checkstyle team if a report is not appropriate.

What do you think?

Thanks

Jacques

Le 04/05/2021 à 10:36, [hidden email] a écrit :

> This is an automated email from the ASF dual-hosted git repository.
>
> jleroux pushed a commit to branch trunk
> in repository https://gitbox.apache.org/repos/asf/ofbiz-framework.git
>
>
> The following commit(s) were added to refs/heads/trunk by this push:
>       new 45f50c9  Improved: Handle remaining checkstyle errors (OFBIZ-12169)
> 45f50c9 is described below
>
> commit 45f50c910d64c12a2601164fbc64a4a02ef1d833
> Author: Jacques Le Roux <[hidden email]>
> AuthorDate: Tue May 4 10:34:52 2021 +0200
>
>      Improved: Handle remaining checkstyle errors (OFBIZ-12169)
>      
>      This handles the "'(' should be on the previous line?" rule of MethodParamPad
>      
>      Actually not completely, because there are still some cases that can't be
>      handled by changing code, notably when a casting is necessary. I believe there
>      is an error in checkstyle code. I'll discuss that in dev ML before sending
>      checkstyle team a report.
> ---
>   .../java/org/apache/ofbiz/accounting/invoice/InvoiceServices.java  | 4 ++--
>   .../java/org/apache/ofbiz/order/order/OrderReturnServices.java     | 3 ++-
>   .../java/org/apache/ofbiz/order/shoppingcart/ShoppingCart.java     | 4 ++--
>   build.gradle                                                       | 2 +-
>   .../common/src/main/java/org/apache/ofbiz/common/CommonEvents.java | 7 +++----
>   .../main/java/org/apache/ofbiz/webapp/control/ContextFilter.java   | 2 +-
>   .../src/main/java/org/apache/ofbiz/webtools/WebToolsServices.java  | 2 +-
>   7 files changed, 12 insertions(+), 12 deletions(-)
>
> diff --git a/applications/accounting/src/main/java/org/apache/ofbiz/accounting/invoice/InvoiceServices.java b/applications/accounting/src/main/java/org/apache/ofbiz/accounting/invoice/InvoiceServices.java
> index 9c956e8..7ee585a 100644
> --- a/applications/accounting/src/main/java/org/apache/ofbiz/accounting/invoice/InvoiceServices.java
> +++ b/applications/accounting/src/main/java/org/apache/ofbiz/accounting/invoice/InvoiceServices.java
> @@ -457,8 +457,8 @@ public class InvoiceServices {
>                   Map<String, Object> createInvoiceItemContext = new HashMap<>();
>                   createInvoiceItemContext.put("invoiceId", invoiceId);
>                   createInvoiceItemContext.put("invoiceItemSeqId", invoiceItemSeqId);
> -                createInvoiceItemContext.put("invoiceItemTypeId", getInvoiceItemType(delegator, (orderItem.getString("orderItemTypeId")),
> -                        (product == null ? null : product.getString("productTypeId")), invoiceType, "INV_FPROD_ITEM"));
> +                createInvoiceItemContext.put("invoiceItemTypeId", getInvoiceItemType(delegator, orderItem.getString("orderItemTypeId"),
> +                        product == null ? null : product.getString("productTypeId"), invoiceType, "INV_FPROD_ITEM"));
>                   createInvoiceItemContext.put("description", orderItem.get("itemDescription"));
>                   createInvoiceItemContext.put("quantity", billingQuantity);
>                   createInvoiceItemContext.put("amount", billingAmount);
> diff --git a/applications/order/src/main/java/org/apache/ofbiz/order/order/OrderReturnServices.java b/applications/order/src/main/java/org/apache/ofbiz/order/order/OrderReturnServices.java
> index d8b469b..17f43b2 100644
> --- a/applications/order/src/main/java/org/apache/ofbiz/order/order/OrderReturnServices.java
> +++ b/applications/order/src/main/java/org/apache/ofbiz/order/order/OrderReturnServices.java
> @@ -1325,7 +1325,8 @@ public class OrderReturnServices {
>                           orderPayPrefDetails.put("orderPaymentPreference", orderPayPref);
>                           orderPayPrefDetails.put("availableTotal", orderPayPrefAvailableTotal);
>                           if (prefSplitMap.containsKey(paymentMethodTypeId)) {
> -                            (prefSplitMap.get(paymentMethodTypeId)).add(orderPayPrefDetails);
> +                            List<Map<String, Object>> paymentMethodTypeIds = prefSplitMap.get(paymentMethodTypeId);
> +                            paymentMethodTypeIds.add(orderPayPrefDetails);
>                           } else {
>                               prefSplitMap.put(paymentMethodTypeId, UtilMisc.toList(orderPayPrefDetails));
>                           }
> diff --git a/applications/order/src/main/java/org/apache/ofbiz/order/shoppingcart/ShoppingCart.java b/applications/order/src/main/java/org/apache/ofbiz/order/shoppingcart/ShoppingCart.java
> index d8e9885..1fff629 100644
> --- a/applications/order/src/main/java/org/apache/ofbiz/order/shoppingcart/ShoppingCart.java
> +++ b/applications/order/src/main/java/org/apache/ofbiz/order/shoppingcart/ShoppingCart.java
> @@ -1759,8 +1759,8 @@ public class ShoppingCart implements Iterable<ShoppingCartItem>, Serializable {
>           this.orderAttributes.clear();
>  
>           // clear the additionalPartyRole Map
> -        for (Map.Entry<String, List<String>> me : this.additionalPartyRole.entrySet()) {
> -            ((LinkedList<String>) me.getValue()).clear();
> +        for (Entry<String, List<String>> me : this.additionalPartyRole.entrySet()) {
> +            me.getValue().clear();
>           }
>           this.additionalPartyRole.clear();
>  
> diff --git a/build.gradle b/build.gradle
> index c2168af..840ed8b 100644
> --- a/build.gradle
> +++ b/build.gradle
> @@ -333,7 +333,7 @@ checkstyle {
>       // the sum of errors found last time it was changed after using the
>       // ‘checkstyle’ tool present in the framework and in the official
>       // plugins.
> -    tasks.checkstyleMain.maxErrors = 127
> +    tasks.checkstyleMain.maxErrors = 116
>       // Currently there are a lot of errors so we need to temporarily
>       // hide them to avoid polluting the terminal output.
>       showViolations = false
> diff --git a/framework/common/src/main/java/org/apache/ofbiz/common/CommonEvents.java b/framework/common/src/main/java/org/apache/ofbiz/common/CommonEvents.java
> index debfbcf..0940e37 100644
> --- a/framework/common/src/main/java/org/apache/ofbiz/common/CommonEvents.java
> +++ b/framework/common/src/main/java/org/apache/ofbiz/common/CommonEvents.java
> @@ -275,7 +275,7 @@ public class CommonEvents {
>           Map<String, List<String>> uiLabelObject = null;
>           if (UtilValidate.isNotEmpty(jsonString)) {
>               JSON json = JSON.from(jsonString);
> -            uiLabelObject = UtilGenerics.<Map<String, List<String>>> cast(json.toObject(Map.class));
> +            uiLabelObject = UtilGenerics.cast(json.toObject(Map.class));
>           }
>           if (UtilValidate.isEmpty(uiLabelObject)) {
>               Debug.logError("No resource and labels found in JSON string: " + jsonString, MODULE);
> @@ -307,7 +307,7 @@ public class CommonEvents {
>           Map<String, String> uiLabelObject = null;
>           if (UtilValidate.isNotEmpty(jsonString)) {
>               JSON json = JSON.from(jsonString);
> -            uiLabelObject = UtilGenerics.<Map<String, String>> cast(json.toObject(Map.class));
> +            uiLabelObject = UtilGenerics.cast(json.toObject(Map.class));
>           }
>           if (UtilValidate.isEmpty(uiLabelObject)) {
>               Debug.logError("No resource and labels found in JSON string: " + jsonString, MODULE);
> @@ -404,8 +404,7 @@ public class CommonEvents {
>                   charGraphics.setFont(textFont);
>  
>                   int charX = (int) (0.5 * charDim - 0.5 * charWidth);
> -                charGraphics.drawString("" + captchaCode.charAt(i), charX,
> -                        ((charDim - fontMetrics.getAscent()) / 2 + fontMetrics.getAscent()));
> +                charGraphics.drawString("" + captchaCode.charAt(i), charX, (charDim - fontMetrics.getAscent()) / 2 + fontMetrics.getAscent());
>  
>                   float x = horizMargin + spacePerChar * (i) - charDim / 2.0f;
>                   int y = ((height - charDim) / 2);
> diff --git a/framework/webapp/src/main/java/org/apache/ofbiz/webapp/control/ContextFilter.java b/framework/webapp/src/main/java/org/apache/ofbiz/webapp/control/ContextFilter.java
> index 4a363c5..69f89b1 100644
> --- a/framework/webapp/src/main/java/org/apache/ofbiz/webapp/control/ContextFilter.java
> +++ b/framework/webapp/src/main/java/org/apache/ofbiz/webapp/control/ContextFilter.java
> @@ -142,7 +142,7 @@ public class ContextFilter implements Filter {
>                           GenericValue tenant = EntityQuery.use(baseDelegator).from("Tenant").where("tenantId", tenantId).queryOne();
>                           String initialPath = tenant.getString("initialPath");
>                           if (UtilValidate.isNotEmpty(initialPath) && !"/".equals(initialPath)) {
> -                            ((HttpServletResponse) response).sendRedirect(initialPath);
> +                            httpResponse.sendRedirect(initialPath);
>                               return;
>                           }
>                       }
> diff --git a/framework/webtools/src/main/java/org/apache/ofbiz/webtools/WebToolsServices.java b/framework/webtools/src/main/java/org/apache/ofbiz/webtools/WebToolsServices.java
> index d67652e..5339e0f 100644
> --- a/framework/webtools/src/main/java/org/apache/ofbiz/webtools/WebToolsServices.java
> +++ b/framework/webtools/src/main/java/org/apache/ofbiz/webtools/WebToolsServices.java
> @@ -304,7 +304,7 @@ public class WebToolsServices {
>                   lastUnprocessedFilesCount = unprocessedFiles.size();
>                   messages.add("---------------------------------------");
>                   messages.add(UtilProperties.getMessage(RESOURCE, "EntityImportSucceededNumberFile", UtilMisc.toMap("succeeded",
> -                        (initialListSize - lastUnprocessedFilesCount), "total", initialListSize), locale));
> +                        initialListSize - lastUnprocessedFilesCount, "total", initialListSize), locale));
>                   messages.add(UtilProperties.getMessage(RESOURCE, "EntityImportFailedNumberFile", UtilMisc.toMap("failed",
>                           lastUnprocessedFilesCount, "total", initialListSize), locale));
>                   messages.add("---------------------------------------");
Reply | Threaded
Open this post in threaded view
|

Re: [ofbiz-framework] branch trunk updated: Improved: Handle remaining checkstyle errors (OFBIZ-12169)

Jacques Le Roux
Administrator
Hi,

I send a bug report: Whitespaces at the head of line before a cast and after an instanceof or an "enhanced for" results to "'(' should be on the
previous line"error · Issue #10012 · checkstyle/checkstyle (github.com) <https://github.com/checkstyle/checkstyle/issues/10012>

BTW, I have compared our checkstyle.xml[0] with the one for Sun from the checkstyle team[1] and they look quite dissimilar. At least at 1st glance
comparing with WinMerge. And when using Checkstyle CLI there are much more errors reported with the Sun config from checkstyle team[1]. As code
quality also matter, I believe we should later check that, not a priority of course...

[0] in config/checkstyle, initially created by Mathieu at https://issues.apache.org/jira/browse/OFBIZ-11251
[1] https://raw.githubusercontent.com/checkstyle/checkstyle/master/src/main/resources/sun_checks.xml

HTH

Jacques

Le 04/05/2021 à 19:52, [hidden email] a écrit :

> Hi Team,
>
> I have been working on https://checkstyle.sourceforge.io/config_whitespace.html#MethodParamPad
>
> I could fix few issues in code. But there are still some cases that can't be handled by changing code, notably when a casting is necessary, and not
> only.
>
> I believe there is an error in checkstyle code. If you look at the URL above and the errors reported, you will see that the errors don't belong to
> the tokens to check.
>
> We can use this solution
> https://stackoverflow.com/questions/45109089/checkstyle-can-i-change-the-should-be-on-the-previous-line-rule-of-meth#answer-45109414
> But I believe we should rather discuss with checkstyle team if a report is not appropriate.
>
> What do you think?
>
> Thanks
>
> Jacques
>
> Le 04/05/2021 à 10:36, [hidden email] a écrit :
>> This is an automated email from the ASF dual-hosted git repository.
>>
>> jleroux pushed a commit to branch trunk
>> in repository https://gitbox.apache.org/repos/asf/ofbiz-framework.git
>>
>>
>> The following commit(s) were added to refs/heads/trunk by this push:
>>       new 45f50c9  Improved: Handle remaining checkstyle errors (OFBIZ-12169)
>> 45f50c9 is described below
>>
>> commit 45f50c910d64c12a2601164fbc64a4a02ef1d833
>> Author: Jacques Le Roux <[hidden email]>
>> AuthorDate: Tue May 4 10:34:52 2021 +0200
>>
>>      Improved: Handle remaining checkstyle errors (OFBIZ-12169)
>>           This handles the "'(' should be on the previous line?" rule of MethodParamPad
>>           Actually not completely, because there are still some cases that can't be
>>      handled by changing code, notably when a casting is necessary. I believe there
>>      is an error in checkstyle code. I'll discuss that in dev ML before sending
>>      checkstyle team a report.
>> ---
>> .../java/org/apache/ofbiz/accounting/invoice/InvoiceServices.java | 4 ++--
>> .../java/org/apache/ofbiz/order/order/OrderReturnServices.java | 3 ++-
>> .../java/org/apache/ofbiz/order/shoppingcart/ShoppingCart.java | 4 ++--
>> build.gradle | 2 +-
>> .../common/src/main/java/org/apache/ofbiz/common/CommonEvents.java | 7 +++----
>> .../main/java/org/apache/ofbiz/webapp/control/ContextFilter.java | 2 +-
>> .../src/main/java/org/apache/ofbiz/webtools/WebToolsServices.java | 2 +-
>>   7 files changed, 12 insertions(+), 12 deletions(-)
>>
>> diff --git a/applications/accounting/src/main/java/org/apache/ofbiz/accounting/invoice/InvoiceServices.java
>> b/applications/accounting/src/main/java/org/apache/ofbiz/accounting/invoice/InvoiceServices.java
>> index 9c956e8..7ee585a 100644
>> --- a/applications/accounting/src/main/java/org/apache/ofbiz/accounting/invoice/InvoiceServices.java
>> +++ b/applications/accounting/src/main/java/org/apache/ofbiz/accounting/invoice/InvoiceServices.java
>> @@ -457,8 +457,8 @@ public class InvoiceServices {
>>                   Map<String, Object> createInvoiceItemContext = new HashMap<>();
>>                   createInvoiceItemContext.put("invoiceId", invoiceId);
>> createInvoiceItemContext.put("invoiceItemSeqId", invoiceItemSeqId);
>> - createInvoiceItemContext.put("invoiceItemTypeId", getInvoiceItemType(delegator, (orderItem.getString("orderItemTypeId")),
>> -                        (product == null ? null : product.getString("productTypeId")), invoiceType, "INV_FPROD_ITEM"));
>> + createInvoiceItemContext.put("invoiceItemTypeId", getInvoiceItemType(delegator, orderItem.getString("orderItemTypeId"),
>> +                        product == null ? null : product.getString("productTypeId"), invoiceType, "INV_FPROD_ITEM"));
>>                   createInvoiceItemContext.put("description", orderItem.get("itemDescription"));
>>                   createInvoiceItemContext.put("quantity", billingQuantity);
>>                   createInvoiceItemContext.put("amount", billingAmount);
>> diff --git a/applications/order/src/main/java/org/apache/ofbiz/order/order/OrderReturnServices.java
>> b/applications/order/src/main/java/org/apache/ofbiz/order/order/OrderReturnServices.java
>> index d8b469b..17f43b2 100644
>> --- a/applications/order/src/main/java/org/apache/ofbiz/order/order/OrderReturnServices.java
>> +++ b/applications/order/src/main/java/org/apache/ofbiz/order/order/OrderReturnServices.java
>> @@ -1325,7 +1325,8 @@ public class OrderReturnServices {
>> orderPayPrefDetails.put("orderPaymentPreference", orderPayPref);
>> orderPayPrefDetails.put("availableTotal", orderPayPrefAvailableTotal);
>>                           if (prefSplitMap.containsKey(paymentMethodTypeId)) {
>> - (prefSplitMap.get(paymentMethodTypeId)).add(orderPayPrefDetails);
>> +                            List<Map<String, Object>> paymentMethodTypeIds = prefSplitMap.get(paymentMethodTypeId);
>> + paymentMethodTypeIds.add(orderPayPrefDetails);
>>                           } else {
>> prefSplitMap.put(paymentMethodTypeId, UtilMisc.toList(orderPayPrefDetails));
>>                           }
>> diff --git a/applications/order/src/main/java/org/apache/ofbiz/order/shoppingcart/ShoppingCart.java
>> b/applications/order/src/main/java/org/apache/ofbiz/order/shoppingcart/ShoppingCart.java
>> index d8e9885..1fff629 100644
>> --- a/applications/order/src/main/java/org/apache/ofbiz/order/shoppingcart/ShoppingCart.java
>> +++ b/applications/order/src/main/java/org/apache/ofbiz/order/shoppingcart/ShoppingCart.java
>> @@ -1759,8 +1759,8 @@ public class ShoppingCart implements Iterable<ShoppingCartItem>, Serializable {
>>           this.orderAttributes.clear();
>>             // clear the additionalPartyRole Map
>> -        for (Map.Entry<String, List<String>> me : this.additionalPartyRole.entrySet()) {
>> -            ((LinkedList<String>) me.getValue()).clear();
>> +        for (Entry<String, List<String>> me : this.additionalPartyRole.entrySet()) {
>> +            me.getValue().clear();
>>           }
>>           this.additionalPartyRole.clear();
>>   diff --git a/build.gradle b/build.gradle
>> index c2168af..840ed8b 100644
>> --- a/build.gradle
>> +++ b/build.gradle
>> @@ -333,7 +333,7 @@ checkstyle {
>>       // the sum of errors found last time it was changed after using the
>>       // ‘checkstyle’ tool present in the framework and in the official
>>       // plugins.
>> -    tasks.checkstyleMain.maxErrors = 127
>> +    tasks.checkstyleMain.maxErrors = 116
>>       // Currently there are a lot of errors so we need to temporarily
>>       // hide them to avoid polluting the terminal output.
>>       showViolations = false
>> diff --git a/framework/common/src/main/java/org/apache/ofbiz/common/CommonEvents.java
>> b/framework/common/src/main/java/org/apache/ofbiz/common/CommonEvents.java
>> index debfbcf..0940e37 100644
>> --- a/framework/common/src/main/java/org/apache/ofbiz/common/CommonEvents.java
>> +++ b/framework/common/src/main/java/org/apache/ofbiz/common/CommonEvents.java
>> @@ -275,7 +275,7 @@ public class CommonEvents {
>>           Map<String, List<String>> uiLabelObject = null;
>>           if (UtilValidate.isNotEmpty(jsonString)) {
>>               JSON json = JSON.from(jsonString);
>> -            uiLabelObject = UtilGenerics.<Map<String, List<String>>> cast(json.toObject(Map.class));
>> +            uiLabelObject = UtilGenerics.cast(json.toObject(Map.class));
>>           }
>>           if (UtilValidate.isEmpty(uiLabelObject)) {
>>               Debug.logError("No resource and labels found in JSON string: " + jsonString, MODULE);
>> @@ -307,7 +307,7 @@ public class CommonEvents {
>>           Map<String, String> uiLabelObject = null;
>>           if (UtilValidate.isNotEmpty(jsonString)) {
>>               JSON json = JSON.from(jsonString);
>> -            uiLabelObject = UtilGenerics.<Map<String, String>> cast(json.toObject(Map.class));
>> +            uiLabelObject = UtilGenerics.cast(json.toObject(Map.class));
>>           }
>>           if (UtilValidate.isEmpty(uiLabelObject)) {
>>               Debug.logError("No resource and labels found in JSON string: " + jsonString, MODULE);
>> @@ -404,8 +404,7 @@ public class CommonEvents {
>>                   charGraphics.setFont(textFont);
>>                     int charX = (int) (0.5 * charDim - 0.5 * charWidth);
>> -                charGraphics.drawString("" + captchaCode.charAt(i), charX,
>> -                        ((charDim - fontMetrics.getAscent()) / 2 + fontMetrics.getAscent()));
>> +                charGraphics.drawString("" + captchaCode.charAt(i), charX, (charDim - fontMetrics.getAscent()) / 2 + fontMetrics.getAscent());
>>                     float x = horizMargin + spacePerChar * (i) - charDim / 2.0f;
>>                   int y = ((height - charDim) / 2);
>> diff --git a/framework/webapp/src/main/java/org/apache/ofbiz/webapp/control/ContextFilter.java
>> b/framework/webapp/src/main/java/org/apache/ofbiz/webapp/control/ContextFilter.java
>> index 4a363c5..69f89b1 100644
>> --- a/framework/webapp/src/main/java/org/apache/ofbiz/webapp/control/ContextFilter.java
>> +++ b/framework/webapp/src/main/java/org/apache/ofbiz/webapp/control/ContextFilter.java
>> @@ -142,7 +142,7 @@ public class ContextFilter implements Filter {
>>                           GenericValue tenant = EntityQuery.use(baseDelegator).from("Tenant").where("tenantId", tenantId).queryOne();
>>                           String initialPath = tenant.getString("initialPath");
>>                           if (UtilValidate.isNotEmpty(initialPath) && !"/".equals(initialPath)) {
>> -                            ((HttpServletResponse) response).sendRedirect(initialPath);
>> + httpResponse.sendRedirect(initialPath);
>>                               return;
>>                           }
>>                       }
>> diff --git a/framework/webtools/src/main/java/org/apache/ofbiz/webtools/WebToolsServices.java
>> b/framework/webtools/src/main/java/org/apache/ofbiz/webtools/WebToolsServices.java
>> index d67652e..5339e0f 100644
>> --- a/framework/webtools/src/main/java/org/apache/ofbiz/webtools/WebToolsServices.java
>> +++ b/framework/webtools/src/main/java/org/apache/ofbiz/webtools/WebToolsServices.java
>> @@ -304,7 +304,7 @@ public class WebToolsServices {
>>                   lastUnprocessedFilesCount = unprocessedFiles.size();
>> messages.add("---------------------------------------");
>> messages.add(UtilProperties.getMessage(RESOURCE, "EntityImportSucceededNumberFile", UtilMisc.toMap("succeeded",
>> -                        (initialListSize - lastUnprocessedFilesCount), "total", initialListSize), locale));
>> +                        initialListSize - lastUnprocessedFilesCount, "total", initialListSize), locale));
>> messages.add(UtilProperties.getMessage(RESOURCE, "EntityImportFailedNumberFile", UtilMisc.toMap("failed",
>>                           lastUnprocessedFilesCount, "total", initialListSize), locale));
>> messages.add("---------------------------------------");

Reply | Threaded
Open this post in threaded view
|

Re: [ofbiz-framework] branch trunk updated: Improved: Handle remaining checkstyle errors (OFBIZ-12169)

Jacques Le Roux
Administrator
Hi,

Thanks to nmancus1's help at https://github.com/checkstyle/checkstyle/issues/10012, I finally don't know if it's a checkstyle config issue or a bug in
checkstyle. I doubt it's the later.

As I wrote there I'm unsure about what to do. Maybe the best is to keep the current checkstyle config and "fix" the "issues" by hand. A bit tedious
but consistent with our checkstyle history.

What do you think?

Thanks

Jacques

Le 17/05/2021 à 11:51, Jacques Le Roux a écrit :

> Hi,
>
> I send a bug report: Whitespaces at the head of line before a cast and after an instanceof or an "enhanced for" results to "'(' should be on the
> previous line"error · Issue #10012 · checkstyle/checkstyle (github.com) <https://github.com/checkstyle/checkstyle/issues/10012>
>
> BTW, I have compared our checkstyle.xml[0] with the one for Sun from the checkstyle team[1] and they look quite dissimilar. At least at 1st glance
> comparing with WinMerge. And when using Checkstyle CLI there are much more errors reported with the Sun config from checkstyle team[1]. As code
> quality also matter, I believe we should later check that, not a priority of course...
>
> [0] in config/checkstyle, initially created by Mathieu at https://issues.apache.org/jira/browse/OFBIZ-11251
> [1] https://raw.githubusercontent.com/checkstyle/checkstyle/master/src/main/resources/sun_checks.xml
>
> HTH
>
> Jacques
>
> Le 04/05/2021 à 19:52, [hidden email] a écrit :
>> Hi Team,
>>
>> I have been working on https://checkstyle.sourceforge.io/config_whitespace.html#MethodParamPad
>>
>> I could fix few issues in code. But there are still some cases that can't be handled by changing code, notably when a casting is necessary, and not
>> only.
>>
>> I believe there is an error in checkstyle code. If you look at the URL above and the errors reported, you will see that the errors don't belong to
>> the tokens to check.
>>
>> We can use this solution
>> https://stackoverflow.com/questions/45109089/checkstyle-can-i-change-the-should-be-on-the-previous-line-rule-of-meth#answer-45109414
>> But I believe we should rather discuss with checkstyle team if a report is not appropriate.
>>
>> What do you think?
>>
>> Thanks
>>
>> Jacques
>>
>> Le 04/05/2021 à 10:36, [hidden email] a écrit :
>>> This is an automated email from the ASF dual-hosted git repository.
>>>
>>> jleroux pushed a commit to branch trunk
>>> in repository https://gitbox.apache.org/repos/asf/ofbiz-framework.git
>>>
>>>
>>> The following commit(s) were added to refs/heads/trunk by this push:
>>>       new 45f50c9  Improved: Handle remaining checkstyle errors (OFBIZ-12169)
>>> 45f50c9 is described below
>>>
>>> commit 45f50c910d64c12a2601164fbc64a4a02ef1d833
>>> Author: Jacques Le Roux <[hidden email]>
>>> AuthorDate: Tue May 4 10:34:52 2021 +0200
>>>
>>>      Improved: Handle remaining checkstyle errors (OFBIZ-12169)
>>>           This handles the "'(' should be on the previous line?" rule of MethodParamPad
>>>           Actually not completely, because there are still some cases that can't be
>>>      handled by changing code, notably when a casting is necessary. I believe there
>>>      is an error in checkstyle code. I'll discuss that in dev ML before sending
>>>      checkstyle team a report.
>>> ---
>>> .../java/org/apache/ofbiz/accounting/invoice/InvoiceServices.java | 4 ++--
>>> .../java/org/apache/ofbiz/order/order/OrderReturnServices.java | 3 ++-
>>> .../java/org/apache/ofbiz/order/shoppingcart/ShoppingCart.java | 4 ++--
>>> build.gradle | 2 +-
>>> .../common/src/main/java/org/apache/ofbiz/common/CommonEvents.java | 7 +++----
>>> .../main/java/org/apache/ofbiz/webapp/control/ContextFilter.java | 2 +-
>>> .../src/main/java/org/apache/ofbiz/webtools/WebToolsServices.java | 2 +-
>>>   7 files changed, 12 insertions(+), 12 deletions(-)
>>>
>>> diff --git a/applications/accounting/src/main/java/org/apache/ofbiz/accounting/invoice/InvoiceServices.java
>>> b/applications/accounting/src/main/java/org/apache/ofbiz/accounting/invoice/InvoiceServices.java
>>> index 9c956e8..7ee585a 100644
>>> --- a/applications/accounting/src/main/java/org/apache/ofbiz/accounting/invoice/InvoiceServices.java
>>> +++ b/applications/accounting/src/main/java/org/apache/ofbiz/accounting/invoice/InvoiceServices.java
>>> @@ -457,8 +457,8 @@ public class InvoiceServices {
>>>                   Map<String, Object> createInvoiceItemContext = new HashMap<>();
>>>                   createInvoiceItemContext.put("invoiceId", invoiceId);
>>> createInvoiceItemContext.put("invoiceItemSeqId", invoiceItemSeqId);
>>> - createInvoiceItemContext.put("invoiceItemTypeId", getInvoiceItemType(delegator, (orderItem.getString("orderItemTypeId")),
>>> -                        (product == null ? null : product.getString("productTypeId")), invoiceType, "INV_FPROD_ITEM"));
>>> + createInvoiceItemContext.put("invoiceItemTypeId", getInvoiceItemType(delegator, orderItem.getString("orderItemTypeId"),
>>> +                        product == null ? null : product.getString("productTypeId"), invoiceType, "INV_FPROD_ITEM"));
>>>                   createInvoiceItemContext.put("description", orderItem.get("itemDescription"));
>>>                   createInvoiceItemContext.put("quantity", billingQuantity);
>>>                   createInvoiceItemContext.put("amount", billingAmount);
>>> diff --git a/applications/order/src/main/java/org/apache/ofbiz/order/order/OrderReturnServices.java
>>> b/applications/order/src/main/java/org/apache/ofbiz/order/order/OrderReturnServices.java
>>> index d8b469b..17f43b2 100644
>>> --- a/applications/order/src/main/java/org/apache/ofbiz/order/order/OrderReturnServices.java
>>> +++ b/applications/order/src/main/java/org/apache/ofbiz/order/order/OrderReturnServices.java
>>> @@ -1325,7 +1325,8 @@ public class OrderReturnServices {
>>> orderPayPrefDetails.put("orderPaymentPreference", orderPayPref);
>>> orderPayPrefDetails.put("availableTotal", orderPayPrefAvailableTotal);
>>>                           if (prefSplitMap.containsKey(paymentMethodTypeId)) {
>>> - (prefSplitMap.get(paymentMethodTypeId)).add(orderPayPrefDetails);
>>> +                            List<Map<String, Object>> paymentMethodTypeIds = prefSplitMap.get(paymentMethodTypeId);
>>> + paymentMethodTypeIds.add(orderPayPrefDetails);
>>>                           } else {
>>> prefSplitMap.put(paymentMethodTypeId, UtilMisc.toList(orderPayPrefDetails));
>>>                           }
>>> diff --git a/applications/order/src/main/java/org/apache/ofbiz/order/shoppingcart/ShoppingCart.java
>>> b/applications/order/src/main/java/org/apache/ofbiz/order/shoppingcart/ShoppingCart.java
>>> index d8e9885..1fff629 100644
>>> --- a/applications/order/src/main/java/org/apache/ofbiz/order/shoppingcart/ShoppingCart.java
>>> +++ b/applications/order/src/main/java/org/apache/ofbiz/order/shoppingcart/ShoppingCart.java
>>> @@ -1759,8 +1759,8 @@ public class ShoppingCart implements Iterable<ShoppingCartItem>, Serializable {
>>>           this.orderAttributes.clear();
>>>             // clear the additionalPartyRole Map
>>> -        for (Map.Entry<String, List<String>> me : this.additionalPartyRole.entrySet()) {
>>> -            ((LinkedList<String>) me.getValue()).clear();
>>> +        for (Entry<String, List<String>> me : this.additionalPartyRole.entrySet()) {
>>> +            me.getValue().clear();
>>>           }
>>>           this.additionalPartyRole.clear();
>>>   diff --git a/build.gradle b/build.gradle
>>> index c2168af..840ed8b 100644
>>> --- a/build.gradle
>>> +++ b/build.gradle
>>> @@ -333,7 +333,7 @@ checkstyle {
>>>       // the sum of errors found last time it was changed after using the
>>>       // ‘checkstyle’ tool present in the framework and in the official
>>>       // plugins.
>>> -    tasks.checkstyleMain.maxErrors = 127
>>> +    tasks.checkstyleMain.maxErrors = 116
>>>       // Currently there are a lot of errors so we need to temporarily
>>>       // hide them to avoid polluting the terminal output.
>>>       showViolations = false
>>> diff --git a/framework/common/src/main/java/org/apache/ofbiz/common/CommonEvents.java
>>> b/framework/common/src/main/java/org/apache/ofbiz/common/CommonEvents.java
>>> index debfbcf..0940e37 100644
>>> --- a/framework/common/src/main/java/org/apache/ofbiz/common/CommonEvents.java
>>> +++ b/framework/common/src/main/java/org/apache/ofbiz/common/CommonEvents.java
>>> @@ -275,7 +275,7 @@ public class CommonEvents {
>>>           Map<String, List<String>> uiLabelObject = null;
>>>           if (UtilValidate.isNotEmpty(jsonString)) {
>>>               JSON json = JSON.from(jsonString);
>>> -            uiLabelObject = UtilGenerics.<Map<String, List<String>>> cast(json.toObject(Map.class));
>>> +            uiLabelObject = UtilGenerics.cast(json.toObject(Map.class));
>>>           }
>>>           if (UtilValidate.isEmpty(uiLabelObject)) {
>>>               Debug.logError("No resource and labels found in JSON string: " + jsonString, MODULE);
>>> @@ -307,7 +307,7 @@ public class CommonEvents {
>>>           Map<String, String> uiLabelObject = null;
>>>           if (UtilValidate.isNotEmpty(jsonString)) {
>>>               JSON json = JSON.from(jsonString);
>>> -            uiLabelObject = UtilGenerics.<Map<String, String>> cast(json.toObject(Map.class));
>>> +            uiLabelObject = UtilGenerics.cast(json.toObject(Map.class));
>>>           }
>>>           if (UtilValidate.isEmpty(uiLabelObject)) {
>>>               Debug.logError("No resource and labels found in JSON string: " + jsonString, MODULE);
>>> @@ -404,8 +404,7 @@ public class CommonEvents {
>>>                   charGraphics.setFont(textFont);
>>>                     int charX = (int) (0.5 * charDim - 0.5 * charWidth);
>>> -                charGraphics.drawString("" + captchaCode.charAt(i), charX,
>>> -                        ((charDim - fontMetrics.getAscent()) / 2 + fontMetrics.getAscent()));
>>> +                charGraphics.drawString("" + captchaCode.charAt(i), charX, (charDim - fontMetrics.getAscent()) / 2 + fontMetrics.getAscent());
>>>                     float x = horizMargin + spacePerChar * (i) - charDim / 2.0f;
>>>                   int y = ((height - charDim) / 2);
>>> diff --git a/framework/webapp/src/main/java/org/apache/ofbiz/webapp/control/ContextFilter.java
>>> b/framework/webapp/src/main/java/org/apache/ofbiz/webapp/control/ContextFilter.java
>>> index 4a363c5..69f89b1 100644
>>> --- a/framework/webapp/src/main/java/org/apache/ofbiz/webapp/control/ContextFilter.java
>>> +++ b/framework/webapp/src/main/java/org/apache/ofbiz/webapp/control/ContextFilter.java
>>> @@ -142,7 +142,7 @@ public class ContextFilter implements Filter {
>>>                           GenericValue tenant = EntityQuery.use(baseDelegator).from("Tenant").where("tenantId", tenantId).queryOne();
>>>                           String initialPath = tenant.getString("initialPath");
>>>                           if (UtilValidate.isNotEmpty(initialPath) && !"/".equals(initialPath)) {
>>> -                            ((HttpServletResponse) response).sendRedirect(initialPath);
>>> + httpResponse.sendRedirect(initialPath);
>>>                               return;
>>>                           }
>>>                       }
>>> diff --git a/framework/webtools/src/main/java/org/apache/ofbiz/webtools/WebToolsServices.java
>>> b/framework/webtools/src/main/java/org/apache/ofbiz/webtools/WebToolsServices.java
>>> index d67652e..5339e0f 100644
>>> --- a/framework/webtools/src/main/java/org/apache/ofbiz/webtools/WebToolsServices.java
>>> +++ b/framework/webtools/src/main/java/org/apache/ofbiz/webtools/WebToolsServices.java
>>> @@ -304,7 +304,7 @@ public class WebToolsServices {
>>>                   lastUnprocessedFilesCount = unprocessedFiles.size();
>>> messages.add("---------------------------------------");
>>> messages.add(UtilProperties.getMessage(RESOURCE, "EntityImportSucceededNumberFile", UtilMisc.toMap("succeeded",
>>> -                        (initialListSize - lastUnprocessedFilesCount), "total", initialListSize), locale));
>>> +                        initialListSize - lastUnprocessedFilesCount, "total", initialListSize), locale));
>>> messages.add(UtilProperties.getMessage(RESOURCE, "EntityImportFailedNumberFile", UtilMisc.toMap("failed",
>>>                           lastUnprocessedFilesCount, "total", initialListSize), locale));
>>> messages.add("---------------------------------------");
>