Skip to content
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] UseNotifyAllInsteadOfNotify falsely detect a special case with argument: foo.notify(bar) #2577

Closed
wuchiuwong opened this issue Jun 14, 2020 · 1 comment
Labels
a:false-positive PMD flags a piece of code that is not problematic
Milestone

Comments

@wuchiuwong
Copy link

Affects PMD Version:
6.22.0

Rule:
UseNotifyAllInsteadOfNotify

Description:
UseNotifyAllInsteadOfNotify falsely detect a special case with argument: foo.notify(bar)
This rule is implemented through xpath search:

<![CDATA[
//StatementExpression/PrimaryExpression
[PrimarySuffix/Arguments[@Size = '0']]
[
    PrimaryPrefix[
        ./Name[@Image='notify' or ends-with(@Image,'.notify')]
        or ../PrimarySuffix/@Image='notify'
        or (./AllocationExpression and ../PrimarySuffix[@Image='notify'])
    ]
]
]]>

AST analyse result of the code sample:

<StatementExpression FindBoundary='false' Image='' SingleLine='true'>
	<PrimaryExpression FindBoundary='false' Image='' SingleLine='true'>
		<PrimaryPrefix FindBoundary='false' Image='' SingleLine='true' SuperModifier='false' ThisModifier='false'>
			<Name FindBoundary='false' Image='getInjector' SingleLine='true' />
		</PrimaryPrefix>
        <PrimarySuffix ArgumentCount='0' Arguments='true' ArrayDereference='false' FindBoundary='false' Image='' SingleLine='true'>
            <Arguments ArgumentCount='0' BeginColumn='28' BeginLine='420' EndColumn='29' EndLine='420' FindBoundary='false' Image='' SingleLine='true' Size='0' />
        </PrimarySuffix>
        <PrimarySuffix ArgumentCount='-1' Arguments='false' ArrayDereference='false' FindBoundary='false' Image='notify' SingleLine='true' />
        <PrimarySuffix ArgumentCount='3' Arguments='true' ArrayDereference='false' FindBoundary='false' Image='' SingleLine='true'>
            <Arguments ArgumentCount='3' FindBoundary='false' Image='' SingleLine='true' Size='3'>
                <ArgumentList BeginColumn='38' BeginLine='420' EndColumn='73' EndLine='420' FindBoundary='false' Image='' SingleLine='true' Size='3'>
                    ......

In the case getInjector().notify(getCanonicalName(), e.getMessage(), e), the fifth line of the AST analysis result is <PrimarySuffix ArgumentCount='0', which causes detection even when notify() with argument.

Code Sample demonstrating the issue:

getInjector().notify(getCanonicalName(), e.getMessage(), e);

Expected outcome:
false-positive

Running PMD through:
CLI

@wuchiuwong wuchiuwong added the a:bug PMD crashes or fails to analyse a file. label Jun 14, 2020
@oowekyala oowekyala added a:false-positive PMD flags a piece of code that is not problematic and removed a:bug PMD crashes or fails to analyse a file. labels Oct 26, 2020
@oowekyala oowekyala added this to the 7.0.0 milestone Nov 11, 2020
oowekyala added a commit to oowekyala/pmd that referenced this issue Nov 11, 2020
@adangel
Copy link
Member

adangel commented Dec 11, 2020

Fixed via #2899 for PMD 7.

@adangel adangel closed this as completed Dec 11, 2020
@adangel adangel changed the title [java]UseNotifyAllInsteadOfNotify falsely detect a special case with argument: foo.notify(bar) [java] UseNotifyAllInsteadOfNotify falsely detect a special case with argument: foo.notify(bar) Jan 7, 2023
@adangel adangel mentioned this issue Jan 23, 2023
55 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
a:false-positive PMD flags a piece of code that is not problematic
Projects
None yet
Development

No branches or pull requests

3 participants