Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP
Browse files

Fixes #45 were return statements were not counted correctly

If nested functions had return, they were counted in the parent
function too. This is because a simple regexp on the body of the
function was done, therefore counting the child too.
Now, the function parser, instead of just checking whether there
is at least one return, counts them.
  • Loading branch information...
commit b82e70da9891aec651fa3f090150d66bc5e759c9 1 parent 9dcbb07
Patrick Brosset authored
View
15 parsers/fileparser.py
@@ -134,4 +134,19 @@ def get_file_data_from_file(src_file_name):
file_data = get_file_data_from_content("test", content)
assert len(file_data.functions[0].lines.get_code_and_comments_lines()) == 1
+ multi_return = """function parent() {
+ var prop = 0;
+ var child1 = function() {
+ return prop;
+ };
+ var child2 = function() {
+ return prop;
+ }
+ return prop;
+ }"""
+ file_data = get_file_data_from_content("multi", multi_return)
+ assert file_data.functions[0].nb_return == 1
+ assert file_data.functions[1].nb_return == 1
+ assert file_data.functions[2].nb_return == 1
+
print "ALL TESTS OK"
View
10 parsers/functionparser.py
@@ -11,12 +11,12 @@ class FunctionData(visitor.Entity):
- signature: an array of argument names
- body: the text of the body of the function
- variables: all variables declared in the function. Type parsers.variableparser.VariableData
- - has_return: whether the function has at least one return statement
+ - nb_return: number of return statement in the function
- lines: an instance of parsers.lineparser.FileLines
- complexity: an integer showing the cyclomatic complexity of the function (minimum 1)
- identifiers_usage: meant to be used through the get_identifier_usage(name) function to know where a given name is used in the function"""
- def __init__(self, name, body, line_number, signature, start_pos, end_pos, variables, has_return):
+ def __init__(self, name, body, line_number, signature, start_pos, end_pos, variables, nb_return):
super(FunctionData, self).__init__(line_number, start_pos, end_pos)
self.name = name
@@ -25,7 +25,7 @@ def __init__(self, name, body, line_number, signature, start_pos, end_pos, varia
self.lines = None
self.variables = variables
- self.has_return = has_return
+ self.nb_return = nb_return
self.complexity = 1
@@ -81,7 +81,7 @@ def __init__(self):
def add_function(self, name, line_number, signature, start_pos, end_pos, source):
body = FunctionBody(source, start_pos, end_pos)
- function = FunctionData(name, body.inner_body, line_number, signature, body.start_pos, body.end_pos, [], False)
+ function = FunctionData(name, body.inner_body, line_number, signature, body.start_pos, body.end_pos, [], 0)
self.functions.append(function)
def add_var(self, function, name, is_nodejs_require, line_number, start, end):
@@ -128,7 +128,7 @@ def visit_VAR(self, node, source):
def visit_RETURN(self, node, source):
functions = self.get_functions_nesting(node.start)
if len(functions) > 0:
- functions[0].has_return = True
+ functions[0].nb_return += 1
def visit_FUNCTION(self, node, source):
# Named functions only, the getattr returns None if name doesn't exist
View
4 parsers/jsparser.py
@@ -167,7 +167,7 @@ def _walk_node(self, root=None):
handler.add_visitor(function_visitor)
handler.visit()
assert function_visitor.functions[0].complexity == 10
- assert function_visitor.functions[0].has_return == True
+ assert function_visitor.functions[0].nb_return == 1
return_code = """
Apple.prototype.getInfo = function() {
@@ -179,7 +179,7 @@ def _walk_node(self, root=None):
handler.add_visitor(function_visitor)
handler.visit()
assert function_visitor.functions[0].complexity == 1
- assert function_visitor.functions[0].has_return == True
+ assert function_visitor.functions[0].nb_return == 1
content = """
//TODO: do something here
View
9 reviewers/complexity.py
@@ -21,11 +21,10 @@ def review_functions_complexity(self, functions, message_bag):
elif function.complexity > Reviewer.WARN_MAX_COMPLEXITY:
message_bag.add_warning(self, "Function " + function.name + " is getting complex (cyclomatic complexity of " + str(function.complexity) + "). There may be too much logic going on. Think about splitting.", function.line_number)
- returns = re.findall("return ", function.lines.get_whole_code())
- if len(returns) > Reviewer.ERROR_MAX_NB_OF_RETURNS_IN_FUNCTION:
- message_bag.add_error(self, "Function " + function.name + " returns more than " + str(Reviewer.ERROR_MAX_NB_OF_RETURNS_IN_FUNCTION) + " values (" + str(len(returns)) + ").", function.line_number)
- elif len(returns) > Reviewer.WARN_MAX_NB_OF_RETURNS_IN_FUNCTION:
- message_bag.add_warning(self, "Function " + function.name + " returns more than " + str(Reviewer.WARN_MAX_NB_OF_RETURNS_IN_FUNCTION) + " values (" + str(len(returns)) + ").", function.line_number)
+ if function.nb_return > Reviewer.ERROR_MAX_NB_OF_RETURNS_IN_FUNCTION:
+ message_bag.add_error(self, "Function " + function.name + " returns more than " + str(Reviewer.ERROR_MAX_NB_OF_RETURNS_IN_FUNCTION) + " values (" + str(function.nb_return) + ").", function.line_number)
+ elif function.nb_return > Reviewer.WARN_MAX_NB_OF_RETURNS_IN_FUNCTION:
+ message_bag.add_warning(self, "Function " + function.name + " returns more than " + str(Reviewer.WARN_MAX_NB_OF_RETURNS_IN_FUNCTION) + " values (" + str(function.nb_return) + ").", function.line_number)
def review_ifs_complexity(self, lines, message_bag):
for line in lines:
View
16 reviewers/naming.py
@@ -29,7 +29,7 @@ def review_gethasis_function_return(self, functions, message_bag):
for function in functions:
name = function.name
is_gethasis = self.name_starts_with(name, "get") or self.name_starts_with(name, "is") or self.name_starts_with(name, "has")
- if is_gethasis and not function.has_return:
+ if is_gethasis and function.nb_return == 0:
message_bag.add_error(self, "Function " + name + " starts with 'is/has/get'. This usually means a return value is expected, but none was found.", function.line_number);
def review_set_function_arg(self, functions, message_bag):
@@ -104,16 +104,16 @@ def check_set_functions_and_assert_bag_contains(functions, nb_expected_messages,
assert len(bag.errors) == nb_expected_messages, msg
# Check that function starting with gethasis actually returns something
- function1 = attrdict(name="getSomeStuff", line_number=0, has_return=True)
- function2 = attrdict(name="isEmpty", line_number=0, has_return=True)
- function3 = attrdict(name="hasHairs", line_number=0, has_return=True)
+ function1 = attrdict(name="getSomeStuff", line_number=0, nb_return=1)
+ function2 = attrdict(name="isEmpty", line_number=0, nb_return=1)
+ function3 = attrdict(name="hasHairs", line_number=0, nb_return=1)
check_gethasis_functions_and_assert_bag_contains([function1,function2,function3], 0, "Functions start with gethasis and returns something, should not output an error")
# Check that functions starting with is has or get, but not followed by capital letter do not trigger any message
- function4 = attrdict(name="getho", line_number=0, has_return=True)
- function5 = attrdict(name="israel", line_number=0, has_return=False)
- function6 = attrdict(name="has", line_number=0, has_return=False)
- function7 = attrdict(name="hasimut", line_number=0, has_return=True)
+ function4 = attrdict(name="getho", line_number=0, nb_return=1)
+ function5 = attrdict(name="israel", line_number=0, nb_return=0)
+ function6 = attrdict(name="has", line_number=0, nb_return=0)
+ function7 = attrdict(name="hasimut", line_number=0, nb_return=1)
check_gethasis_functions_and_assert_bag_contains([function4,function5,function6,function7], 0, "All these functions start with get/has/is but not camelcased, so should not even look into them")
# Check that function starting with set actually take arguments
View
12 testscripts/expected.py
@@ -1,4 +1,10 @@
results = {
+ "multiplenestedreturn.js": {
+ "messages": [
+ '16 Line is more than 120 character long (135). This is hard to read.',
+ '5 Line is more than 120 character long (123). This is hard to read.'
+ ]
+ },
"usedclassprop3.js": {
"messages": [
'2 Variable a in method Test is declared but never used'
@@ -14,8 +20,7 @@
'3 Class property property2 is initialized but never used'
]
},
- "nested.js": {
- },
+ "nested.js": {},
"emptyfunction.js": {
"messages": [
'2 Function myFunction is empty. Is it really needed?'
@@ -30,7 +35,8 @@
'1 The name of argument c is less than 2 characters (1). This is way too short! Noone will understand what you mean',
"1 Name c doesn't mean anything",
"1 Name c doesn't mean anything",
- "1 Name d doesn't mean anything"
+ "1 Name d doesn't mean anything",
+ '1 Function initVideoplayerWithElement returns more than 2 values (5).'
]
},
"linelength.js": {},
View
20 testscripts/multiplenestedreturn.js
@@ -0,0 +1,20 @@
+function getFallbackMarkup (eventName, delegateId, wrapTarget) {
+ if (aria.core.Browser.isIE) {
+ this.getFallbackMarkup = function (eventName, delegateId, wrapTarget) {
+ wrapTarget = wrapTarget ? "true" : "false";
+ return " on" + eventName + "=\"aria.utils.Delegate.directCall(event, '" + delegateId + "', " + wrapTarget + ", this)\"";
+ }
+ } else {
+ this.getFallbackMarkup = function (eventName, delegateId, wrapTarget) {
+ var calledFunction = "directCall";
+ wrapTarget = wrapTarget ? "true" : "false";
+ if ('mouseleave' == eventName || 'mouseenter' == eventName) {
+ // Mouseleave/enter exists only in IE, we can emulate them in all other browsers
+ eventName = 'mouseleave' == eventName ? 'mouseout' : 'mouseover';
+ calledFunction = "mouseMovement";
+ }
+ return " on" + eventName + "=\"aria.utils.Delegate." + calledFunction + "(event, '" + delegateId + "', " + wrapTarget + ", this)\"";
+ }
+ }
+ return this.getFallbackMarkup(eventName, delegateId, wrapTarget);
+};
Please sign in to comment.
Something went wrong with that request. Please try again.