Increase AvoidNotShortCircuitOperatorsForBooleanCheck code coverage #324

Closed
wants to merge 1 commit into
from

Conversation

Projects
None yet
3 participants
@damianszczepanik
Contributor

damianszczepanik commented Feb 28, 2015

Pull # 324

@coveralls

This comment has been minimized.

Show comment
Hide comment
@coveralls

coveralls Feb 28, 2015

Coverage Status

Coverage decreased (-0.0%) to 97.74% when pulling 0b9645a on damianszczepanik:AvoidNotShortCircuitOperatorsForBooleanCheck into f5497d6 on sevntu-checkstyle:master.

Coverage Status

Coverage decreased (-0.0%) to 97.74% when pulling 0b9645a on damianszczepanik:AvoidNotShortCircuitOperatorsForBooleanCheck into f5497d6 on sevntu-checkstyle:master.

@coveralls

This comment has been minimized.

Show comment
Hide comment
@coveralls

coveralls Feb 28, 2015

Coverage Status

Coverage decreased (-0.0%) to 97.74% when pulling 0b9645a on damianszczepanik:AvoidNotShortCircuitOperatorsForBooleanCheck into f5497d6 on sevntu-checkstyle:master.

Coverage Status

Coverage decreased (-0.0%) to 97.74% when pulling 0b9645a on damianszczepanik:AvoidNotShortCircuitOperatorsForBooleanCheck into f5497d6 on sevntu-checkstyle:master.

@coveralls

This comment has been minimized.

Show comment
Hide comment
@coveralls

coveralls Feb 28, 2015

Coverage Status

Coverage decreased (-0.0%) to 97.74% when pulling 0b9645a on damianszczepanik:AvoidNotShortCircuitOperatorsForBooleanCheck into f5497d6 on sevntu-checkstyle:master.

Coverage Status

Coverage decreased (-0.0%) to 97.74% when pulling 0b9645a on damianszczepanik:AvoidNotShortCircuitOperatorsForBooleanCheck into f5497d6 on sevntu-checkstyle:master.

@romani

This comment has been minimized.

Show comment
Hide comment
@romani

romani Mar 1, 2015

Member

please explain each changes in logic of Check (especially testing for nulls).
Any changes in logic will require manual retesting over big projects. I am not afraid of changes in code and testing but please prove that checks for NULLs are useless.

Member

romani commented Mar 1, 2015

please explain each changes in logic of Check (especially testing for nulls).
Any changes in logic will require manual retesting over big projects. I am not afraid of changes in code and testing but please prove that checks for NULLs are useless.

@@ -115,27 +115,18 @@ public final void visitToken(final DetailAST detailAST)
{
DetailAST currentNode = detailAST;
- while (currentNode != null

This comment has been minimized.

@damianszczepanik

damianszczepanik Mar 1, 2015

Contributor

If you pass any of operators then soon or later you get EXPR as parent and that's why validating against other tokens doesn't make sense. Also null value should not be possible because I imagine that null is not passed to visitiToken() method. Am I right?

@damianszczepanik

damianszczepanik Mar 1, 2015

Contributor

If you pass any of operators then soon or later you get EXPR as parent and that's why validating against other tokens doesn't make sense. Also null value should not be possible because I imagine that null is not passed to visitiToken() method. Am I right?

{
currentNode = currentNode.getParent();
}
- final int type = currentNode.getType();
-
- if (type == TokenTypes.EXPR) {

This comment has been minimized.

@damianszczepanik

damianszczepanik Mar 1, 2015

Contributor

Already checked above

@damianszczepanik

damianszczepanik Mar 1, 2015

Contributor

Already checked above

DetailAST curNode = node;
final List<String> childNames = getSupportedOperandsNames(curNode);
final List<String> booleanVariablesNames = new LinkedList<String>();
- while (curNode != null

This comment has been minimized.

@damianszczepanik

damianszczepanik Mar 1, 2015

Contributor

Can you meet null in hierarchy between EXPR and any of those values? I Believe not and that's why I dropped it.

@damianszczepanik

damianszczepanik Mar 1, 2015

Contributor

Can you meet null in hierarchy between EXPR and any of those values? I Believe not and that's why I dropped it.

@@ -218,7 +207,6 @@ public final boolean isBooleanExpression(final DetailAST node)
}
if (currentNode.getType() == TokenTypes.IDENT
- && currentNode.getParent() != null

This comment has been minimized.

@damianszczepanik

damianszczepanik Mar 1, 2015

Contributor

if currentNode is IDENT it always has not-null value as parent.

@damianszczepanik

damianszczepanik Mar 1, 2015

Contributor

if currentNode is IDENT it always has not-null value as parent.

@romani

This comment has been minimized.

Show comment
Hide comment
@romani

romani Mar 1, 2015

Member

merged as FF
make sense , thanks a lot !

Member

romani commented Mar 1, 2015

merged as FF
make sense , thanks a lot !

@romani romani closed this Mar 1, 2015

@damianszczepanik damianszczepanik deleted the damianszczepanik:AvoidNotShortCircuitOperatorsForBooleanCheck branch Mar 1, 2015

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment