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

[javascript] UnnecessaryBlock FP for function declaration #4129

Closed
naveadkazi opened this issue Sep 19, 2022 · 4 comments
Closed

[javascript] UnnecessaryBlock FP for function declaration #4129

naveadkazi opened this issue Sep 19, 2022 · 4 comments
Labels
a:false-positive PMD flags a piece of code that is not problematic
Milestone

Comments

@naveadkazi
Copy link

Affects PMD Version: Not sure, in codacy it give since 5.0

Rule:

Please provide the rule name and a link to the rule documentation:
https://pmd.github.io/latest/pmd_rules_ecmascript_codestyle.html#unnecessaryblock

Description:
Created a named exported function that returns an object. This function is pushed to codacy.com which uses PMD. Codacy reports it as an unnecessary block. I am not sure if this false positive and what I am missing here.

image

Code Sample demonstrating the issue:


Expected outcome:

PMD reports a violation at line 188 in the screenshot, but that's wrong. That's a false positive.

Running PMD through: Codacy

@naveadkazi naveadkazi added the a:false-positive PMD flags a piece of code that is not problematic label Sep 19, 2022
@naveadkazi
Copy link
Author

EDIT: codacy using PMD : 6.48.0

@naveadkazi
Copy link
Author

I tried running on local environment and it reported as Unnecessary block. But if I move the export statement at end of the file then PMD is not reporting it. The below code is similar to one in the issue description with named export at the end. This is not reporting the issue

const getAddtitonalTagTrackingImpressionObj = (
  profileId,
  cityName,
  tracking,
  isAdditionalTagsAndSlicerAbTest
) => {
  const action = isAdditionalTagsAndSlicerAbTest
    ? `TagShown${tracking}`
    : ADDITIONAL_TAG_NOT_SHOWN_ACTION;
  return {
    gaCat: SEARCH_PAGE_TRACKING_CATEGORY,
    gaActImp: action,
    gaLblImp: `profileId=${profileId}|Cityname=${cityName}`,
    once: true,
    trackImpression: true,
  };
};

export { getAddtitonalTagTrackingImpressionObj };

@oowekyala
Copy link
Member

Related to #2305. Probably export is not supported by Rhino, or at least improperly parsed. The false-positive could probably be fixed by also requiring that the reported block is not a child of the root node (see the rule's xpath definition linked above).

@jsotuyod jsotuyod added the needs:pmd7-revalidation The issue hasn't yet been retested vs PMD 7 and may be stale label Mar 17, 2024
@jsotuyod jsotuyod removed the needs:pmd7-revalidation The issue hasn't yet been retested vs PMD 7 and may be stale label Apr 2, 2024
@jsotuyod jsotuyod added this to the 7.0.0 milestone Apr 2, 2024
@jsotuyod
Copy link
Member

jsotuyod commented Apr 2, 2024

PMD 7.0.0 won't produce a FP here regardless of where the export is located, but indeed, Rhino doesn't properly support that keyword, simply saying the "identifier" is a reserved keyword and producing a bunch of EmptyStatements.

@jsotuyod jsotuyod closed this as completed Apr 2, 2024
@adangel adangel changed the title [ecmascript] function declaration report unnecessary block issue [javascript] UnnecessaryBlock FP for function declaration Oct 24, 2024
adangel added a commit to adangel/pmd that referenced this issue Oct 31, 2024
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