Skip to content

Commit

Permalink
Prevent incorrect wrapping of return statements.
Browse files Browse the repository at this point in the history 
There cannot be a newline between the return keyword and the start of
the return value expression.

Closes #751
  • Loading branch information
@bitwiseman
Glenn Moss authored and bitwiseman committed Jan 22, 2016
1 parent 1c6a198 commit 21eb8fb
Show file tree
Hide file tree
Showing 6 changed files with 157 additions and 2 deletions.
8 changes: 7 additions & 1 deletion js/lib/beautify.js
View file
Expand Up @@ -429,6 +429,7 @@
return out;
}

var newline_restricted_tokens = ['break','contiue','return', 'throw'];
function allow_wrap_or_preserved_newline(force_linewrap) {
force_linewrap = (force_linewrap === undefined) ? false : force_linewrap;

Expand All @@ -440,6 +441,11 @@
if ((opt.preserve_newlines && current_token.wanted_newline) || force_linewrap) {
print_newline(false, true);
} else if (opt.wrap_line_length) {
if (last_type === 'TK_RESERVED' && in_array(flags.last_text, newline_restricted_tokens)) {
// These tokens should never have a newline inserted
// between them and the following expression.
return
}
var proposed_line_length = output.current_line.get_character_count() + current_token.text.length +
(output.space_before_token ? 1 : 0);
if (proposed_line_length >= opt.wrap_line_length) {
Expand Down Expand Up @@ -542,7 +548,7 @@
if (
(last_type === 'TK_RESERVED' && in_array(flags.last_text, ['var', 'let', 'const']) && current_token.type === 'TK_WORD') ||
(last_type === 'TK_RESERVED' && flags.last_text === 'do') ||
(last_type === 'TK_RESERVED' && flags.last_text === 'return' && !current_token.wanted_newline) ||
(last_type === 'TK_RESERVED' && in_array(flags.last_text, ['return', 'throw']) && !current_token.wanted_newline) ||
(last_type === 'TK_RESERVED' && flags.last_text === 'else' && !(current_token.type === 'TK_RESERVED' && current_token.text === 'if')) ||
(last_type === 'TK_END_EXPR' && (previous_flags.mode === MODE.ForInitializer || previous_flags.mode === MODE.Conditional)) ||
(last_type === 'TK_WORD' && flags.mode === MODE.BlockStatement
Expand Down
36 changes: 36 additions & 0 deletions js/test/generated/beautify-javascript-tests.js
View file
Expand Up @@ -2119,6 +2119,8 @@ function run_javascript_tests(test_obj, Urlencoded, js_beautify, html_beautify,
//.............1234567890123456789012345678901234567890123456789012345678901234567890
wrap_input_1=('foo.bar().baz().cucumber((fat && "sassy") || (leans\n&& mean));\n' +
'Test_very_long_variable_name_this_should_never_wrap\n.but_this_can\n' +
'return between_return_and_expression_should_never_wrap.but_this_can\n' +
'throw between_throw_and_expression_should_never_wrap.but_this_can\n' +
'if (wraps_can_occur && inside_an_if_block) that_is_\n.okay();\n' +
'object_literal = {\n' +
' propertx: first_token + 12345678.99999E-6,\n' +
Expand All @@ -2132,6 +2134,8 @@ function run_javascript_tests(test_obj, Urlencoded, js_beautify, html_beautify,
wrap_input_2=('{\n' +
' foo.bar().baz().cucumber((fat && "sassy") || (leans\n&& mean));\n' +
' Test_very_long_variable_name_this_should_never_wrap\n.but_this_can\n' +
' return between_return_and_expression_should_never_wrap.but_this_can\n' +
' throw between_throw_and_expression_should_never_wrap.but_this_can\n' +
' if (wraps_can_occur && inside_an_if_block) that_is_\n.okay();\n' +
' object_literal = {\n' +
' propertx: first_token + 12345678.99999E-6,\n' +
Expand All @@ -2149,6 +2153,8 @@ function run_javascript_tests(test_obj, Urlencoded, js_beautify, html_beautify,
/* expected */
'foo.bar().baz().cucumber((fat && "sassy") || (leans && mean));\n' +
'Test_very_long_variable_name_this_should_never_wrap.but_this_can\n' +
'return between_return_and_expression_should_never_wrap.but_this_can\n' +
'throw between_throw_and_expression_should_never_wrap.but_this_can\n' +
'if (wraps_can_occur && inside_an_if_block) that_is_.okay();\n' +
'object_literal = {\n' +
' propertx: first_token + 12345678.99999E-6,\n' +
Expand All @@ -2164,6 +2170,8 @@ function run_javascript_tests(test_obj, Urlencoded, js_beautify, html_beautify,
/* expected */
'foo.bar().baz().cucumber((fat && "sassy") || (leans && mean));\n' +
'Test_very_long_variable_name_this_should_never_wrap.but_this_can\n' +
'return between_return_and_expression_should_never_wrap.but_this_can\n' +
'throw between_throw_and_expression_should_never_wrap.but_this_can\n' +
'if (wraps_can_occur && inside_an_if_block) that_is_.okay();\n' +
'object_literal = {\n' +
' propertx: first_token + 12345678.99999E-6,\n' +
Expand All @@ -2181,6 +2189,10 @@ function run_javascript_tests(test_obj, Urlencoded, js_beautify, html_beautify,
' "sassy") || (leans && mean));\n' +
'Test_very_long_variable_name_this_should_never_wrap\n' +
' .but_this_can\n' +
'return between_return_and_expression_should_never_wrap\n' +
' .but_this_can\n' +
'throw between_throw_and_expression_should_never_wrap\n' +
' .but_this_can\n' +
'if (wraps_can_occur &&\n' +
' inside_an_if_block) that_is_.okay();\n' +
'object_literal = {\n' +
Expand All @@ -2204,6 +2216,10 @@ function run_javascript_tests(test_obj, Urlencoded, js_beautify, html_beautify,
' (leans && mean));\n' +
'Test_very_long_variable_name_this_should_never_wrap\n' +
' .but_this_can\n' +
'return between_return_and_expression_should_never_wrap\n' +
' .but_this_can\n' +
'throw between_throw_and_expression_should_never_wrap\n' +
' .but_this_can\n' +
'if (wraps_can_occur &&\n' +
' inside_an_if_block) that_is_.okay();\n' +
'object_literal = {\n' +
Expand All @@ -2228,6 +2244,10 @@ function run_javascript_tests(test_obj, Urlencoded, js_beautify, html_beautify,
' (leans && mean));\n' +
' Test_very_long_variable_name_this_should_never_wrap\n' +
' .but_this_can\n' +
' return between_return_and_expression_should_never_wrap\n' +
' .but_this_can\n' +
' throw between_throw_and_expression_should_never_wrap\n' +
' .but_this_can\n' +
' if (wraps_can_occur &&\n' +
' inside_an_if_block) that_is_.okay();\n' +
' object_literal = {\n' +
Expand All @@ -2251,6 +2271,8 @@ function run_javascript_tests(test_obj, Urlencoded, js_beautify, html_beautify,
'foo.bar().baz().cucumber((fat && "sassy") || (leans && mean));\n' +
'Test_very_long_variable_name_this_should_never_wrap\n' +
' .but_this_can\n' +
'return between_return_and_expression_should_never_wrap.but_this_can\n' +
'throw between_throw_and_expression_should_never_wrap.but_this_can\n' +
'if (wraps_can_occur && inside_an_if_block) that_is_\n' +
' .okay();\n' +
'object_literal = {\n' +
Expand All @@ -2268,6 +2290,8 @@ function run_javascript_tests(test_obj, Urlencoded, js_beautify, html_beautify,
'foo.bar().baz().cucumber((fat && "sassy") || (leans && mean));\n' +
'Test_very_long_variable_name_this_should_never_wrap\n' +
' .but_this_can\n' +
'return between_return_and_expression_should_never_wrap.but_this_can\n' +
'throw between_throw_and_expression_should_never_wrap.but_this_can\n' +
'if (wraps_can_occur && inside_an_if_block) that_is_\n' +
' .okay();\n' +
'object_literal = {\n' +
Expand All @@ -2287,6 +2311,10 @@ function run_javascript_tests(test_obj, Urlencoded, js_beautify, html_beautify,
' "sassy") || (leans && mean));\n' +
'Test_very_long_variable_name_this_should_never_wrap\n' +
' .but_this_can\n' +
'return between_return_and_expression_should_never_wrap\n' +
' .but_this_can\n' +
'throw between_throw_and_expression_should_never_wrap\n' +
' .but_this_can\n' +
'if (wraps_can_occur &&\n' +
' inside_an_if_block) that_is_\n' +
' .okay();\n' +
Expand All @@ -2311,6 +2339,10 @@ function run_javascript_tests(test_obj, Urlencoded, js_beautify, html_beautify,
' (leans && mean));\n' +
'Test_very_long_variable_name_this_should_never_wrap\n' +
' .but_this_can\n' +
'return between_return_and_expression_should_never_wrap\n' +
' .but_this_can\n' +
'throw between_throw_and_expression_should_never_wrap\n' +
' .but_this_can\n' +
'if (wraps_can_occur &&\n' +
' inside_an_if_block) that_is_\n' +
' .okay();\n' +
Expand All @@ -2336,6 +2368,10 @@ function run_javascript_tests(test_obj, Urlencoded, js_beautify, html_beautify,
' (leans && mean));\n' +
' Test_very_long_variable_name_this_should_never_wrap\n' +
' .but_this_can\n' +
' return between_return_and_expression_should_never_wrap\n' +
' .but_this_can\n' +
' throw between_throw_and_expression_should_never_wrap\n' +
' .but_this_can\n' +
' if (wraps_can_occur &&\n' +
' inside_an_if_block) that_is_\n' +
' .okay();\n' +
Expand Down
7 changes: 6 additions & 1 deletion python/jsbeautifier/__init__.py
View file
Expand Up @@ -475,6 +475,7 @@ def is_expression(self, mode):
return mode in [MODE.Expression, MODE.ForInitializer, MODE.Conditional]


_newline_restricted_tokens = ['break','contiue','return', 'throw']
def allow_wrap_or_preserved_newline(self, current_token, force_linewrap = False):
# never wrap the first token of a line.
if self.output.just_added_newline():
Expand All @@ -483,6 +484,10 @@ def allow_wrap_or_preserved_newline(self, current_token, force_linewrap = False)
if (self.opts.preserve_newlines and current_token.wanted_newline) or force_linewrap:
self.print_newline(preserve_statement_flags = True)
elif self.opts.wrap_line_length > 0:
if self.last_type == 'TK_RESERVED' and self.flags.last_text in self._newline_restricted_tokens:
# These tokens should never have a newline inserted between
# them and the following expression.
return
proposed_line_length = self.output.current_line.get_character_count() + len(current_token.text)
if self.output.space_before_token:
proposed_line_length += 1
Expand Down Expand Up @@ -565,7 +570,7 @@ def start_of_statement(self, current_token):
if (
(self.last_type == 'TK_RESERVED' and self.flags.last_text in ['var', 'let', 'const'] and current_token.type == 'TK_WORD') \
or (self.last_type == 'TK_RESERVED' and self.flags.last_text== 'do') \
or (self.last_type == 'TK_RESERVED' and self.flags.last_text== 'return' and not current_token.wanted_newline) \
or (self.last_type == 'TK_RESERVED' and self.flags.last_text in ['return', 'throw'] and not current_token.wanted_newline) \
or (self.last_type == 'TK_RESERVED' and self.flags.last_text == 'else' and not (current_token.type == 'TK_RESERVED' and current_token.text == 'if' )) \
or (self.last_type == 'TK_END_EXPR' and (self.previous_flags.mode == MODE.ForInitializer or self.previous_flags.mode == MODE.Conditional)) \
or (self.last_type == 'TK_WORD' and self.flags.mode == MODE.BlockStatement \
Expand Down

0 comments on commit 21eb8fb

Please sign in to comment.