pmd icon indicating copy to clipboard operation
pmd copied to clipboard

[javascript] UnnecessaryBlock - false positives with destructuring assignments

Open bkdotcom opened this issue 5 years ago • 7 comments

Rule: UnnecessaryBlock

Description: Unfortunately I do not know what version of PMD is being used.. PMD is one of the test suites that codacy.com uses..

Code Sample demonstrating the issue:

import { foo } from './someScript.js' // missing semicolon
const { isLoggedIn } = useAuth();  // <--- violation here: unnecessary block

throws "Unnecessary block."

However, the brackets are quite necessary and designate a named import.

bkdotcom avatar Feb 21 '20 16:02 bkdotcom

Weirdly enough the parser produces EmptyStatement nodes for imports. Eg for the example on https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Statements/import

import defaultExport from "module-name";
import * as name from "module-name";
import { export1 } from "module-name";
import { export1 as alias1 } from "module-name";
import { export1 , export2 } from "module-name";
import { foo , bar } from "module-name/path/to/specific/un-exported/file";
import { export1 , export2 as alias2 } from "module-name";
import defaultExport, { export1 } from "module-name";
import defaultExport, * as name from "module-name";
import "module-name";

var promise = import("module-name");
└─ AstRoot
   ├─ EmptyStatement
   ├─ EmptyStatement
   ├─ EmptyStatement
   ├─ EmptyStatement
   ├─ EmptyStatement
   ├─ EmptyStatement
   ├─ EmptyStatement
   ├─ EmptyStatement
   ├─ EmptyStatement
   ├─ EmptyStatement
   └─ VariableDeclaration
      └─ VariableInitializer
         ├─ Name
         └─ FunctionCall
            ├─ Name
            └─ StringLiteral

oowekyala avatar Feb 23 '20 14:02 oowekyala

import seems to be a new feature, that is not supported by the Rhino parser version, we are using. With #699 we plan to update the version with PMD 7. I don't know though, whether Rhino at all supports such imports.

adangel avatar Feb 24 '20 18:02 adangel

I've just tried it with Rhino 1.7.12 and it seems, that import is not supported unfortunately.

adangel avatar Apr 16 '20 14:04 adangel

any news on this? #2906 will fix it?

christian-hawk avatar Feb 25 '21 14:02 christian-hawk

any news on this? #2906 will fix it?

No - see #2906:

The new rhino version still doesn't support import statements (#2305)

adangel avatar Feb 25 '21 16:02 adangel

Import is not the only trigger for this error. It also happens with destructuring assignments where pmd sees the curly brackets as an extra block.

  const { isLoggedIn } = useAuth();

Why is this an issue? Since: PMD 5.0 An unnecessary Block is present. Such Blocks are often used in other languages to introduce a new variable scope. Blocks > do not behave like this in ECMAScipt, and using them can be misleading. Considering removing this unnecessary Block.

simoami avatar Jan 02 '24 06:01 simoami

fixed in #4829

oleksandr-shvets avatar Feb 22 '24 13:02 oleksandr-shvets