-
-
Notifications
You must be signed in to change notification settings - Fork 1.5k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[java] CompareObjectsWithEquals / UseEqualsToCompareStrings - False negatives with fields #2934
Conversation
Fixes false negatives in case fields are referenced with this. Type resolution on PrimaryExpression works well enough to simply check the type.
Generated by 🚫 Danger |
From the logs:
regression tester was unable to download the master baseline from sourceforge.... I hope, that's only a temporary problem. |
Seems like there are some issues atm - see https://sourceforge.net/p/forge/site-support/21502/ I could resolve it now by deleting the baseline and re-uploading it. That triggered the mirroring again and the download worked: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good. I'll merge into master and incorporate these improvements into #2899
...est/resources/net/sourceforge/pmd/lang/java/rule/errorprone/xml/CompareObjectsWithEquals.xml
Show resolved
Hide resolved
* [doc] Update release notes, refs pmd#2834, fixes pmd#2765 * Bump antlr from 4.7 to 4.7.2 * [doc] Update release notes, refs pmd#2842 * [apex] Remove deprecated rules from quickstart.xml, restore Test * [doc] Update release notes, refs pmd#2816, fixes pmd#1713 * [apex] Update apex-jorje to 2020-09-10-5a5192 (Winter 21, Version 50) Fixes pmd#2839 * [apex] Add support and test for parsing SafeNavigationOperator Refs pmd#2839 * [doc] Update release notes, refs pmd#2839 * Provide type information to Visualforce rules Addresses the general issue raised in pmd#1092 This commit removes false positives from expressions in apex tags. The specific use case raised in 1092 isn't reproducible and represents a false negative that will be fixed separately. The existing Visualforce rules don't have any information about the data types referenced in the Visualforce page. This results in false positives when attempting to identify expressions that are vulnerable to XSS attacks. The rules should not warn about XSS attacks when the expression refers to a type such as Integer or Boolean. The VfExpressionTypeVisitor visits the Visualforce page and extracts the datatypes from Salesforce metadata. Data type information can come from either Apex classes or Object Fields. The Salesforce metadata is generally located in a sibling directory of the Visualforce directory. By default the code looks in directories relative to the Visualforce file to find the metadata. The conventional locations for the metadata are "../classes" and "../objects", the user can override this default with other directories if required. * Mark AbstractVfTypedElExpressionRule as abstract * Update ExcessiveImports example code for clarity Since the default minimum is 30, I believe it would make more sense if the comment said "28" instead of "18". * Programming correction in docs Docs of CouplingBetweenObjects has a Programming issue. I also made "something" a method. * Prepare pmd release 6.29.0 * [maven-release-plugin] prepare release pmd_releases/6.29.0 * [maven-release-plugin] prepare for next development iteration * Prepare next development version * (dogfood) Bump pmd from 6.28.0 to 6.29.0 * Upgrade to kotlin 1.4.1 and kotest 4.3.0 * Check XPath expression result for correct type Fixes pmd#1939 * Check result of / expression Refs pmd#1938 * Test remaining cases of pmd#1938 * Document pmd#1938 in migration guide * Update release notes, refs pmd#1939 * [core] Fix XMLRenderer with UTF-16 When using UTF-16 as encoding, the XMLRenderer produced invalid XML: When inserting the linebreak encoded with the given encoding "UTF-16", a BOM was created. This inserted additional characters U+FEFF in the middle of the file, which is not allowed. A partial workaround for this issue would be, to use "UTF-16BE" as encoding instead. This doesn't create a BOM. However, the resulting XML file is then completely without a BOM. * Replace createFactory methods with a builder Deprecate compatibility filter Deprecate methods in RulesetsFactUtils * Rename * Isolate single rule pattern * [doc][skip ci] Update release notes * Deprecate other APIs * Cleanup API around processors * Deprecate some methods of PMD * Deprecate configuration objects * Doc * Rename stuff * Fix bug * PMD warnings * Stop parsing comma-separated paths by default * Checkstyle * Fix broken rule reporting * [ci] Update travis build badge After move to travis-ci.com * Rename ExpressionType, remove google collections Renamed ExpressionType to IdentifierType since this is more accurate. Removed usage of google.collect classes that were causing UnsupportedClassVersionError exception in the Travis CI run. * [ci] Update install-openjdk to always use latest jdk This change will download openjdk from fixed URLs so that we don't need to update the script everytime a new jdk version is relased. * [ci] Display used java version in github action * [ci] Update pmd-tester, use auxclasspath This will test pmd/pmd-regression-tester#72 checkstyle: Add exclude patterns for checkstyle: after compilation, testresources appear now under target/test-classes and they don't need to be analyzed again. Also exclude generated-sources. * [ci] Use ruby 2.7 on travis * [ci] Use new project-list.xml for Danger * [ci] Switch back to pmd/pmd-regression-tester@master * [java] Add tests for UnusedAssignments Refs pmd#2843 * Add back pom.parent.relativePath for pmd-lang-test * Fix release notes format Co-authored-by: Andreas Dangel <andreas.dangel@pmd-code.org> * Configure visualforce to require Java 8 pmd-visualforce depends on pmd-apex. pmd-apex relies on Java 8. This change configures pmd-visualforce to also require Java 8. This is a breaking change that will need to be documented. * [ci] Remove jobs for mac and windows on travis Builds are executed by github actions and don't need to run on travis again. * Exclude languages in AbstractRuleSetFactoryTest Allow subclasses of AbstractRuleSetFactoryTest to filter out languages that show up in the classpath but should not be tested. Change VFTestContstants to final instead of abstract. * Deprecate ASTTypeParameter#getParameterName * Deprecate ASTPackageDeclaration#getPackageNameImage * Update release notes * Replace usages, update reference files * [ci] Update "PMD Release Signing Key" Now Valid till 2021-12-31 http://pool.sks-keyservers.net:11371/pks/lookup?search=0xD0BF1D737C9A1C22&fingerprint=on&op=index https://keys.openpgp.org/vks/v1/by-fingerprint/EBB241A545CB17C87FACB2EBD0BF1D737C9A1C22 * Return a report instead of side-effecting on it I think this will be more compatible with pmd 7. * Rename RuleSetParser to RuleSetLoader * Hide RuleSetLoader fields * Remove try with resources * Rename to loadFromResources * Update release notes * Provide replacement api for getRegisteredRuleSets * [javascript] Add deprecations for EcmascriptParser/Handler Refs pmd#2837 * [doc] Fix release notes * Bump kotest from 4.3.0 to 4.3.1 * [core] Add deprecations for ParametricRuleViolation and ParserOptions#suppressMarker * [modelica] Internalize ModelicaRuleViolationFactory This was missing, all other impls are internal api already * Deprecations for pmd#905 * Initial version of new ci scripts * Fix wget download of openjdk * Use bash * Add test case for pmd#2595 * Fix install-openjdk.sh * Integrate setup secrets into build.sh, add env.gpg, add sourceforge upload * Add workaround for connection timeouts * Make sure to not log secrets while loading * Move includes into inc/ and files into files/, add pmd_doc support * Use github job infos * [core] Modified test cases for TextRenderer to include the RuleName in the generated output * Fix path for maven settings, remove progress-meter while downloading * Implement check-environment * [core] Modified output of TextRenderer to include the RuleName in the generated output * [core] Removed unnecessary explicit casting of Integer to String * Add regression-tester for updating baseline * Add debugging hints * [core] Refactored generation of Text output to use constants defining the 3 different types of text seperators: small '-'; medium ':\t' and large '\t-\t' to increase readability and to standardise the text output. * Remove potential old stale bundle config * Copy files from old .travis directory * [core] Corrected constant naming from SEPERATOR to SEPARATOR. * Integrate danger for pull requests * [core] Changed output for a violating rule to put the RuleName in front of the description after receiving feedback on the PR. * Update pmd-core/src/main/java/net/sourceforge/pmd/renderers/TextRenderer.java Co-authored-by: Clément Fournier <clement.fournier76@gmail.com> * Add docker for testing * Remove unsupported --no-progress-meter for curl * Add unzip and timezone * Try disable http pooling * Try to set wagon options via MAVEN_OPTS * Another try with maven.wagon.http.pool=false * Store type information in AST instead of map Store the IdentifierType on ASTIdentifier node instead of in a separate map. Use the existing TypeResolution pattern to configure the visitor instead deriving from an abstract rule. Changed ParserOptions to extend AbstractPropertySource with the ability to override the defaults via environment variables. * Take out windows build It's always failing to download dependencies * Try again with wagon options * Add troubleshooting * Move visitor to VfParser#parse LanguageVersionHandler#getTypeResolutionFacade is deprecated. Moved the VfExpressionTypeVisitor creation and execution to VfParser#parse instead. ParsingOptionsTest located in pmd-test wasn't running previously because it was in the src/main hierarchy. Moved this test into the src/test hierarchy and consolidated the methods from the similarly named class from pmd-core. * Resolve maven dependencies before building This tries to solve build timeouts while downloading dependencies. Also the job timeout for PRs is 30 minutes - if it takes longer, something is wrong. * Enable pull-requests workflow * There are no secrets in pull requests * Enable windows build again * Externalize tokens for Danger, use correct base branch ref * Add logging groups * Fix pull requests build - correctly extract data from event * Move install-openjdk into main script, add check-environment * Increase fetch depth so that danger finds the HEAD commit of the PR * Fetch more commits of the PR for danger * Fix unnecessary throws clause * Describe workaround for failing downloads * Implement release builds * Fix maven-dependencies * Add missing include install-openjdk * Adjust integration test for new folder .ci * Add missing tool zip * Don't check rule docs on pull requests This check fails under Windows * Update test instructions * Apparently spring-framework doesn't build anymore without maven central * Add sonar and coveralls jobs * Add vendor/bundle to cache * Add github actions badge * Remove travis config * Fix docu * Simplify constructor of JavaTypeDefinitionSimple * Restrict caching to classes loaded by the bootstrap loader * Fix errors * Load fewer class names * Don't cache enclosing class (pretty rare to hit it) * Make overload without varargs * Disable debug logging * Remove cache entirely * Update release notes, refs pmd#2920 * [ci] Fix builds for Windows / MacOS Add missing include for logger * [ci] Windows needs maven_dependencies_resolve * Use the same build script for Windows+MacOS as for PRs * Also install java7 for PRs (that was missing from the old travis solution) * Run coveralls and sonar in parallel to win/macos after linux * [ci] Only install java7 on Linux, add GITHUB_TOKEN for sonar * Correct Annotation Array Initialiation indents from checkstyle #8083 * [scala] adding support for CPD-ON and CPD-OFF special comments * [scala] fixing support for CPD-ON and CPD-OFF special comments - minor fix ups as per PR comments - comments are skipped and no longer tokenised * [java] CompareObjectsWithEquals / UseEqualsToCompareStrings - Fix FN Fixes false negatives in case fields are referenced with this. Type resolution on PrimaryExpression works well enough to simply check the type. * Fix typo: "an accessor" not "a" * [doc] Update release notes, refs pmd#2934 * [java] Consider comparison with null literal as valid * [cs] CPD: fix issue where ignoring using directives could not be disabled via the GUI after enabling it * [java] Avoid false positives with literals * Use new dokka javadoc path * [java] CompareObjectsWithEquals - whitelist java.lang.class and this * [java] CompareObjectsWithEquals: Fix more false positives * Update the way nodes with data are identified Changed method for how the Visualforce strings are reconstructed from the AST. The previous implementation had incorrect assumptions about the structure of the AST. Added tests to more thoroughly test these situations. Changed name of IdentifierType to DataType. This information can be stored on either ASTIdentifier or ASTLiteral nodes. Changes based on PR feedgack: - Restored ParserOptionsTest in order to avoid binary compatibilty issues. - Changed ParserOptions to contain a PropertySource instead of extending AbtractPropertySource. * [java] Catch additional TypeNotPresentExceptions / LinkageErrors These exceptions are indications of an incomplete auxclasspath but should not fail PMD entirely. Incomplete auxclasspath can still result in invalid violations, either false positives or false negatives. * Bump m-pmd-p from 3.13.0 to 3.14.0 * [doc] Update release notes, refs pmd#2938 * [scala] ScalaTokenizer: Make inner classes static * [doc] Update release notes, refs pmd#2929, fixes pmd#2480 * [doc] Update text renderer format example * [doc] Update release notes, refs pmd#2914, fixes pmd#1961 * Update pmd-java/src/main/resources/category/java/bestpractices.xml Co-authored-by: Igor Moreno <igormoreno@gmail.com> * [doc] Update release notes, refs pmd#2936 * Handle TNPE and LinkageE when loading type params * Cleanup languages to skip * Make the parser options properties private * Remove AbstractPropertySource equals/hashCode * Use interface instead of deprecated abstract class * Don't create a vf language module per parser options * Remove some unchecked warnings * Fix tests * Replace checked exception with wrapper * Update pmd-doc module to use newer apis * Fix bug with sub-report not being merged into global report * Fix ant tests Report was being rendered mutliple times * [ci] Upload/download baseline for regression tester from pmd-code.org Baseline is downloaded before executing regression tester, so that regression tester just reuses it and doesn't try to download from sourceforge. We'll only upload to pmd-code.org. Sourceforge is commented out for now. * [ci] Update regression tester * Remove workaround for pre-downloading baseline and add baseline-download-url option * Remove unnecessary travis_wait * Add error-recovery flag * [ci] Fetch more commits from master for regression testing PRs That only needed for pmd/7.0.x PRs * Fix javadoc * Add option to ignore sequences of literals In some cases, code may include sequences of literals that represent lists or tables of constants, such as lookup tables. Large sequences of these (particularly parts with many zeroes) will be identified by CPD as duplicates, but in practice, these are not the types of duplicates that are considered interesting. This introduces a new option for CPD (--ignore-literal-sequences) that ignores these sequences of literals, in a very similar way to how using directives for C# can already be skipped as well. For now, this functionality is restricted to C#, but it could be added for other languages as well. * [ci] Avoid mixing caches * Update release notes Refs pmd#2940 * Add doc in cpd.md * Update release notes, refs pmd#2945 * Update CPD md page for pmd#2929 * Add links * [ci] Fix do-release.sh script * [java] CompareObjectsWithEquals: only allow this with equals (pmd#2934) * [ci] Update pmdtester to 1.1.0 * Fix Html DataType This DataType does not need to be escaped, it is always escaped by the server. * Add support for C++14 binary literals * Add some doc * Update release notes * Small comment and doc edits * Update release notes, refs pmd#2962 * [ci] Fix build badge * Bump org.codehaus.groovy:groovy from 2.4.7 to 2.4.21 * Use Github Discussions for Q&A * Rename RuleSetLoadException * Remove unneeded suppresswarnings in AbstractPMDProcessor * Prepare pmd release 6.30.0 * [maven-release-plugin] prepare release pmd_releases/6.30.0 * [maven-release-plugin] prepare for next development iteration * Prepare next development version * [ci] Fix build in trunk (pmd#2967) The same problem would have occurred for the release build. * [ci] Use ruby 2.7 for release build (pmd#2967) * [ci] Install needed gems before rendering release notes (pmd#2967) * [doc] Fix release notes * [ci] Fix creating regression tester baseline for release builds (pmd#2967) Co-authored-by: Andreas Dangel <andreas.dangel@pmd-code.org> Co-authored-by: Andreas Dangel <andreas.dangel@adangel.org> Co-authored-by: Jeff Bartolotta <jbartolotta@salesforce.com> Co-authored-by: Gustavo Krieger <gustavopcassol@gmail.com> Co-authored-by: Clément Fournier <clement.fournier76@gmail.com> Co-authored-by: Gunther Schrijvers <gunther@familieschrijvers.be> Co-authored-by: GuntherSchrijvers <56870283+GuntherSchrijvers@users.noreply.github.com> Co-authored-by: abhishek-kumar09 <abhimait1909@gmail.com> Co-authored-by: Andy Robinson <andy.robinson01@bbc.co.uk> Co-authored-by: Igor Moreno <igormoreno@gmail.com> Co-authored-by: Maikel Steneker <maikel.steneker@tiobe.com>
Describe the PR
Fixes false negatives in case fields are referenced with this.
Type resolution on PrimaryExpression works well enough to
simply check the type.
CompareObjectsWithEquals had the same problem. It was easier to rewrite this rule as a XPath rule.
Note: I changed one test case, which was actually wrong.
Example
Related issues
Ready?
./mvnw clean verify
passes (checked automatically by travis)