jmeter-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Philippe Mouawad <philippe.moua...@gmail.com>
Subject Re: Error Prone (static analysis tool for Java that catches common programming mistakes) results
Date Mon, 31 Oct 2016 21:09:33 GMT
Hi Antonio,
My answers below.
Do you have the full report ?
Thanks

On Thu, Oct 27, 2016 at 2:46 PM, Antonio Gomes Rodrigues <ra0077@gmail.com>
wrote:

> Hi all,
>
> I have run Error Prone in JMeter source code
>
>
> **** It have found  2 [ReferenceEquality] Comparison using reference
> equality instead of value equality
> http://errorprone.info/bugpattern/ReferenceEquality
>
>
>     [javac]
> C:\Util\0_perso\TestError\trunk\src\core\org\apache\jmeter\samplers\
> SampleSaveConfiguration.java:600:
> warning: [ReferenceEquality] Comparison using reference equality instead of
> value equality
>     [javac]             stringValues = s.delimiter == delimiter ||
> (delimiter != null && delimiter.equals(s.delimiter));
>     [javac]                                        ^
>     [javac]     (see http://errorprone.info/bugpattern/ReferenceEquality)
>     [javac]   Did you mean 'stringValues = Objects.equals(s.delimiter,
> delimiter) || (delimiter != null && delimiter.equals(s.delimiter));' or
> 'stringValues = s.delimiter.equals(delimiter) || (delimiter != null &&
> delimiter.equals(s.delimiter));'?
>     [javac]
> C:\Util\0_perso\TestError\trunk\src\core\org\apache\jmeter\samplers\
> SampleSaveConfiguration.java:604:
> warning: [ReferenceEquality] Comparison using reference equality instead of
> value equality
>     [javac]             complexValues = s.formatter == formatter ||
> (formatter != null && formatter.equals(s.formatter));
>     [javac]                                         ^
>     [javac]     (see http://errorprone.info/bugpattern/ReferenceEquality)
>     [javac]   Did you mean 'complexValues = Objects.equals(s.formatter,
> formatter) || (formatter != null && formatter.equals(s.formatter));' or
> 'complexValues = s.formatter.equals(formatter) || (formatter != null &&
> formatter.equals(s.formatter));'?
>
>
> I don't think there's a bug, but we could use Objects.equals to reduce
syntax

>
> ****And some [ClassNewInstance] Class.newInstance() bypasses exception
> checking; prefer getConstructor().newInstance()
> http://errorprone.info/bugpattern/ClassNewInstance
>
>     [javac]
> C:\Util\0_perso\TestError\trunk\src\core\org\apache\
> jmeter\testelement\property\MapProperty.java:119:
> warning: [ClassNewInstance] Class.newInstance() bypasses exception
> checking; prefer getConstructor().newInstance()
>     [javac]             Map<String, JMeterProperty> newCol =
> value.getClass().newInstance();
>
> [javac]
> ^
>     [javac]     (see http://errorprone.info/bugpattern/ClassNewInstance)
>     [javac]   Did you mean 'Map<String, JMeterProperty> newCol =
> value.getClass().getConstructor().newInstance();'?
>

We should fix that at least for Java 9.
We have something like 60 occurences

>
>
> ****1 Synchronizing on non-final fields is not safe: if the field is ever
> updated, different threads may end up locking on different objects
> http://errorprone.info/bugpattern/SynchronizeOnNonFinalField
>
>     [javac]
> C:\Util\0_perso\TestError\trunk\src\core\org\apache\
> jmeter\reporters\Summariser.java:190:
> warning: [SynchronizeOnNonFinalField] Synchronizing on non-final fields is
> not safe: if the field is ever updated, different threads may end up
> locking on different objects.
>     [javac]         synchronized (myTotals) {
>     [javac]                      ^
>     [javac]     (see
> http://errorprone.info/bugpattern/SynchronizeOnNonFinalField)
>

False positive for me in this case but others review is welcome.

>
> ****1 error [ChainingConstructorIgnoresParameter] The called constructor
> accepts a parameter with the same name and type as one of its caller's
> parameters, but its caller doesn't pass that parameter to it.  It's likely
> that it was intended to.
> http://errorprone.info/bugpattern/ChainingConstructorIgnoresParameter
>
>     [javac]
> C:\Util\0_perso\TestError\trunk\src\core\org\apache\
> jmeter\gui\util\FilePanel.java:44:
> error: [ChainingConstructorIgnoresParameter] The called constructor
> accepts
> a parameter with the same name and type as one of its caller's parameters,
> but its caller doesn't pass that parameter to it.  It's likely that it was
> intended to.
>     [javac]         this(title, (String) null, false);
>     [javac]             ^
>     [javac]     (see
> http://errorprone.info/bugpattern/ChainingConstructorIgnoresParameter)
>     [javac]   Did you mean 'this(title, filetype, false);'?
>

Good catch.
It affected Html Assertion and IncludeController

>
>
> Anybody have already use this tool and check if results ara false positive?
>

Seems to be a good bug detector.

>
> Error Prone : https://github.com/google/error-prone
>
>
> Antonio
>



-- 
Cordialement.
Philippe Mouawad.

Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message