Skip to content

SONARJS-67 Rule: Variables should not be declared and then immediately returned or thrown#429

Merged
vilchik-elena merged 3 commits intomasterfrom
SONARJS-67
Dec 23, 2016
Merged

SONARJS-67 Rule: Variables should not be declared and then immediately returned or thrown#429
vilchik-elena merged 3 commits intomasterfrom
SONARJS-67

Conversation

@vilchik-elena
Copy link
Copy Markdown
Contributor

No description provided.

],
'project:dojo-1.8.1/dojo/dojo.js.uncompressed.js':[
2638,
15035,
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@vilchik-elena Why did we lose some issues?


if (statements.size() > 1) {

StatementTree lastButOneStatement = statements.get(statements.size() - 2);
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

penultimateStatement ? :D

if (initializedBindingElementTree.left().is(Kind.BINDING_IDENTIFIER)) {
String name = ((IdentifierTree) initializedBindingElementTree.left()).name();

if (lastStatementReturnsVariable(lastStatement, name)) {
Copy link
Copy Markdown

@ghost ghost Dec 23, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would rename these methods returnsVariableInStatement and throwsVariableInStatement, in order to have the difference of naming at the begining of the name rather that in the middle.

if (lastStatement.is(Kind.RETURN_STATEMENT)) {
ReturnStatementTree returnStatement = (ReturnStatementTree) lastStatement;

if (returnStatement.expression() != null && returnStatement.expression().is(Kind.IDENTIFIER_REFERENCE)) {
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would factorize this part of the method, in order to have a method isVariable(ExpressionTree expr, variableName) that you can call from this method and from lastStatementThrowsVariable, instead of having two time nearly exactly the same thing.

@vilchik-elena vilchik-elena merged commit 9783228 into master Dec 23, 2016
@vilchik-elena vilchik-elena deleted the SONARJS-67 branch January 10, 2017 10:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants