Remove expand-strict and fix brace tests

While removing expand-strict, found that brace tests were uneven. Made all tests appear for all three settings, and adjusted for each. There are still some minor odd behaviors around empty braces, but they are non-breaking. Closes beautifier#236, beautifier#237
bitwiseman · Apr 18, 2013 · 11e50ba · 11e50ba
1 parent d02d83d
commit 11e50ba
Show file tree

Hide file tree

Showing 7 changed files with 205 additions and 92 deletions.
diff --git a/README.md b/README.md
@@ -8,17 +8,17 @@ as well as deobfuscate scripts processed by
 [javascriptobfuscator.com](http://javascriptobfuscator.com/).
 
 # Usage
-You can beautify javascript using JS Beautifier in your web browser, or on the command-line using node.js or python. 
+You can beautify javascript using JS Beautifier in your web browser, or on the command-line using node.js or python.
 
-## Web Browser 
-Open [jsbeautifier.org](http://jsbeautifier.org/).  Options are available via the UI. 
+## Web Browser
+Open [jsbeautifier.org](http://jsbeautifier.org/).  Options are available via the UI.
 
 ## Python
 To beautify using python:
 
 ```bash
 $ pip install jsbeautifier
-$ js-beautify file.js 
+$ js-beautify file.js
 ```
 
 Beautified output goes to `stdout`.
@@ -89,7 +89,7 @@ Beautifier Options:
   -p, --preserve-newlines       Preserve existing line-breaks (--no-preserve-newlines disables)
   -m, --max-preserve-newlines   Maximum number of line-breaks to be preserved in one chunk [10]
   -j, --jslint-happy            Enable jslint-stricter mode
-  -b, --brace-style             [collapse|expand|end-expand|expand-strict] ["collapse"]
+  -b, --brace-style             [collapse|expand|end-expand] ["collapse"]
   -B, --break-chained-methods   Break chained method calls across subsequent lines
   -k, --keep-array-indentation  Preserve array indentation
   -x, --unescape-strings        Decode printable characters encoded in xNN notation

diff --git a/index.html b/index.html
@@ -309,7 +309,6 @@
             <option value="collapse">Braces with control statement</option>
             <option value="expand">Braces on own line</option>
             <option value="end-expand">End braces on own line</option>
-            <!-- option value="expand-strict">Braces always on own line (not recommended)</option -->
           </select>
 
           <p style="margin:6px 0 0 0">HTML &lt;style&gt;, &lt;script&gt; formatting:</p><select id="indent-scripts">

diff --git a/js/lib/beautify.js b/js/lib/beautify.js
@@ -53,18 +53,9 @@
             ---------------------------------
              function ()      function()
 
-    brace_style (default "collapse") - "collapse" | "expand" | "end-expand" | "expand-strict"
+    brace_style (default "collapse") - "collapse" | "expand" | "end-expand"
             put braces on the same line as control statements (default), or put braces on own line (Allman / ANSI style), or just put end braces on own line.
 
-            expand-strict: put brace on own line even in such cases:
-
-                var a =
-                {
-                    a: 5,
-                    b: 6
-                }
-            This mode may break your scripts - e.g "return { a: 1 }" will be broken into two lines, so beware.
-
     space_before_conditional (default true) - should the space before conditional statement be added, "if(true)" vs "if (true)",
 
     unescape_strings (default false) - should printable characters in strings encoded in \xNN notation be unescaped, "example" vs "\x65\x78\x61\x6d\x70\x6c\x65"
@@ -174,6 +165,12 @@
         }
         opt.brace_style = options.brace_style ? options.brace_style : (opt.brace_style ? opt.brace_style : "collapse");
 
+        // graceful handling of deprecated option
+        if (opt.brace_style === "expand-strict") {
+            opt.brace_style = "expand";
+        }
+
+
         opt.indent_size = options.indent_size ? parseInt(options.indent_size, 10) : 4;
         opt.indent_char = options.indent_char ? options.indent_char : ' ';
         opt.preserve_newlines = (options.preserve_newlines === undefined) ? true : options.preserve_newlines;
@@ -955,19 +952,17 @@
             set_mode(MODE.BlockStatement);
 
             var empty_braces = is_next('}');
-
-            if (opt.brace_style === "expand-strict") {
-                if (!empty_braces) {
-                    print_newline();
-                }
-            } else if (opt.brace_style === "expand") {
-                if (last_type !== 'TK_OPERATOR') {
-                    if (last_type === 'TK_EQUALS' ||
-                        (is_special_word (flags.last_text) && flags.last_text !== 'else')) {
+            var empty_anonymous_function = empty_braces && flags.last_word === 'function' &&
+                last_type === 'TK_END_EXPR';
+
+            if (opt.brace_style === "expand") {
+                if (last_type !== 'TK_OPERATOR' &&
+                    (empty_anonymous_function ||
+                        last_type === 'TK_EQUALS' ||
+                        (is_special_word (flags.last_text) && flags.last_text !== 'else'))) {
                         output_space_before_token = true;
-                    } else {
-                        print_newline();
-                    }
+                } else {
+                    print_newline();
                 }
             } else { // collapse
                 if (last_type !== 'TK_OPERATOR' && last_type !== 'TK_START_EXPR') {
@@ -998,13 +993,15 @@
                 restore_mode();
             }
             restore_mode();
-            if (opt.brace_style === "expand" || opt.brace_style === "expand-strict") {
-                if  (last_type !== 'TK_START_BLOCK') {
+            var empty_braces = last_type === 'TK_START_BLOCK';
+
+            if (opt.brace_style === "expand") {
+                if  (!empty_braces) {
                     print_newline();
                 }
             } else {
                 // skip {}
-                if (last_type !== 'TK_START_BLOCK') {
+                if (!empty_braces) {
                     if (is_array(flags.mode) && opt.keep_array_indentation) {
                         // we REALLY need a newline here, but newliner would skip that
                         opt.keep_array_indentation = false;
@@ -1113,7 +1110,7 @@
                 if (!in_array(token_text, ['else', 'catch', 'finally'])) {
                     prefix = 'NEWLINE';
                 } else {
-                    if (opt.brace_style === "expand" || opt.brace_style === "end-expand" || opt.brace_style === "expand-strict") {
+                    if (opt.brace_style === "expand" || opt.brace_style === "end-expand") {
                         prefix = 'NEWLINE';
                     } else {
                         prefix = 'SPACE';
@@ -1152,7 +1149,7 @@
             }
 
             if (in_array(token_text, ['else', 'catch', 'finally'])) {
-                if (last_type !== 'TK_END_BLOCK' || opt.brace_style === "expand" || opt.brace_style === "end-expand" || opt.brace_style === "expand-strict") {
+                if (last_type !== 'TK_END_BLOCK' || opt.brace_style === "expand" || opt.brace_style === "end-expand") {
                     print_newline();
                 } else {
                     trim_output(true);

diff --git a/js/lib/cli.js b/js/lib/cli.js
@@ -50,6 +50,7 @@ var fs = require('fs'),
         "preserve_newlines": Boolean,
         "max_preserve_newlines": Number,
         "jslint_happy": Boolean,
+        // TODO: expand-string is obsolete, now identical to expand.  Remove in future version
         "brace_style": ["collapse", "expand", "end-expand", "expand-strict"],
         "break_chained_methods": Boolean,
         "keep_array_indentation": Boolean,
@@ -181,7 +182,7 @@ function usage(err) {
         msg.push('  -p, --preserve-newlines       Preserve line-breaks (--no-preserve-newlines disables)');
         msg.push('  -m, --max-preserve-newlines   Number of line-breaks to be preserved in one chunk [10]');
         msg.push('  -j, --jslint-happy            Enable jslint-stricter mode');
-        msg.push('  -b, --brace-style             [collapse|expand|end-expand|expand-strict] ["collapse"]');
+        msg.push('  -b, --brace-style             [collapse|expand|end-expand] ["collapse"]');
         msg.push('  -B, --break-chained-methods   Break chained method calls across subsequent lines');
         msg.push('  -k, --keep-array-indentation  Preserve array indentation');
         msg.push('  -x, --unescape-strings        Decode printable characters encoded in xNN notation');

diff --git a/js/test/beautify-tests.js b/js/test/beautify-tests.js
@@ -436,59 +436,114 @@ function run_beautifier_tests(test_obj, Urlencoded, js_beautify)
         opts.brace_style = 'expand';
 
         bt('//case 1\nif (a == 1)\n{}\n//case 2\nelse if (a == 2)\n{}');
+        bt('if(1){2}else{3}', "if (1)\n{\n    2\n}\nelse\n{\n    3\n}");
+        bt('try{a();}catch(b){c();}catch(d){}finally{e();}',
+            "try\n{\n    a();\n}\ncatch (b)\n{\n    c();\n}\ncatch (d)\n{}\nfinally\n{\n    e();\n}");
+        bt('if(a){b();}else if(c) foo();',
+            "if (a)\n{\n    b();\n}\nelse if (c) foo();");
+        bt("if (a) {\n// comment\n}else{\n// comment\n}",
+            "if (a)\n{\n    // comment\n}\nelse\n{\n    // comment\n}"); // if/else statement with empty body
+        bt('if (x) {y} else { if (x) {y}}',
+            'if (x)\n{\n    y\n}\nelse\n{\n    if (x)\n    {\n        y\n    }\n}');
+        bt('if (a)\n{\nb;\n}\nelse\n{\nc;\n}',
+            'if (a)\n{\n    b;\n}\nelse\n{\n    c;\n}');
+        test_fragment('    /*\n* xx\n*/\n// xx\nif (foo) {\n    bar();\n}',
+                      '    /*\n     * xx\n     */\n    // xx\n    if (foo)\n    {\n        bar();\n    }');
+        bt('if (foo)\n{}\nelse /regex/.test();');
+        bt('if (foo) /regex/.test();');
         bt('if (a)\n{\nb;\n}\nelse\n{\nc;\n}', 'if (a)\n{\n    b;\n}\nelse\n{\n    c;\n}');
         test_fragment('if (foo) {', 'if (foo)\n{');
         test_fragment('foo {', 'foo\n{');
-        test_fragment('return {', 'return {'); // return needs the brace. maybe something else as well: feel free to report.
+        test_fragment('return {', 'return {'); // return needs the brace.
         test_fragment('return /* inline */ {', 'return /* inline */ {');
         // test_fragment('return\n{', 'return\n{'); // can't support this?, but that's an improbable and extreme case anyway.
         test_fragment('return;\n{', 'return;\n{');
         bt("throw {}");
         bt("throw {\n    foo;\n}");
-
-        bt('if (a)\n{\nb;\n}\nelse\n{\nc;\n}', 'if (a)\n{\n    b;\n}\nelse\n{\n    c;\n}');
         bt('var foo = {}');
-        bt('if (a)\n{\nb;\n}\nelse\n{\nc;\n}', 'if (a)\n{\n    b;\n}\nelse\n{\n    c;\n}');
-        test_fragment('if (foo) {', 'if (foo)\n{');
-        test_fragment('foo {', 'foo\n{');
-        test_fragment('return {', 'return {'); // return needs the brace. maybe something else as well: feel free to report.
-        test_fragment('return /* inline */ {', 'return /* inline */ {');
-        // test_fragment('return\n{', 'return\n{'); // can't support this?, but that's an improbable and extreme case anyway.
-        test_fragment('return;\n{', 'return;\n{');
+        bt('if (foo) bar();\nelse break');
+        bt('function x() {\n    foo();\n}zzz', 'function x()\n{\n    foo();\n}\nzzz');
+        bt('a: do {} while (); xxx', 'a: do {} while ();\nxxx');
+        bt('var a = new function();');
+        bt('var a = new function() {};');
+        bt('var a = new function a()\n    {};');
+        test_fragment('new function');
 
 
         opts.brace_style = 'collapse';
 
         bt('//case 1\nif (a == 1) {}\n//case 2\nelse if (a == 2) {}');
+        bt('if(1){2}else{3}', "if (1) {\n    2\n} else {\n    3\n}");
+        bt('try{a();}catch(b){c();}catch(d){}finally{e();}',
+             "try {\n    a();\n} catch (b) {\n    c();\n} catch (d) {} finally {\n    e();\n}");
+        bt('if(a){b();}else if(c) foo();',
+            "if (a) {\n    b();\n} else if (c) foo();");
+        bt("if (a) {\n// comment\n}else{\n// comment\n}",
+            "if (a) {\n    // comment\n} else {\n    // comment\n}"); // if/else statement with empty body
+        bt('if (x) {y} else { if (x) {y}}',
+            'if (x) {\n    y\n} else {\n    if (x) {\n        y\n    }\n}');
+        bt('if (a)\n{\nb;\n}\nelse\n{\nc;\n}',
+            'if (a) {\n    b;\n} else {\n    c;\n}');
+        test_fragment('    /*\n* xx\n*/\n// xx\nif (foo) {\n    bar();\n}',
+                      '    /*\n     * xx\n     */\n    // xx\n    if (foo) {\n        bar();\n    }');
+        bt('if (foo) {} else /regex/.test();');
+        bt('if (foo) /regex/.test();');
         bt('if (a)\n{\nb;\n}\nelse\n{\nc;\n}', 'if (a) {\n    b;\n} else {\n    c;\n}');
         test_fragment('if (foo) {', 'if (foo) {');
         test_fragment('foo {', 'foo {');
-        test_fragment('return {', 'return {'); // return needs the brace. maybe something else as well: feel free to report.
+        test_fragment('return {', 'return {'); // return needs the brace.
         test_fragment('return /* inline */ {', 'return /* inline */ {');
         // test_fragment('return\n{', 'return\n{'); // can't support this?, but that's an improbable and extreme case anyway.
         test_fragment('return;\n{', 'return; {');
-
+        bt("throw {}");
+        bt("throw {\n    foo;\n}");
+        bt('var foo = {}');
         bt('if (foo) bar();\nelse break');
         bt('function x() {\n    foo();\n}zzz', 'function x() {\n    foo();\n}\nzzz');
         bt('a: do {} while (); xxx', 'a: do {} while ();\nxxx');
-
         bt('var a = new function();');
+        bt('var a = new function() {};');
+        bt('var a = new function a() {};');
         test_fragment('new function');
 
         opts.brace_style = "end-expand";
 
         bt('//case 1\nif (a == 1) {}\n//case 2\nelse if (a == 2) {}');
         bt('if(1){2}else{3}', "if (1) {\n    2\n}\nelse {\n    3\n}");
-        bt('try{a();}catch(b){c();}finally{d();}', "try {\n    a();\n}\ncatch (b) {\n    c();\n}\nfinally {\n    d();\n}");
-        bt('if(a){b();}else if(c) foo();', "if (a) {\n    b();\n}\nelse if (c) foo();");
-        bt("if (a) {\n// comment\n}else{\n// comment\n}", "if (a) {\n    // comment\n}\nelse {\n    // comment\n}"); // if/else statement with empty body
-        bt('if (x) {y} else { if (x) {y}}', 'if (x) {\n    y\n}\nelse {\n    if (x) {\n        y\n    }\n}');
+        bt('try{a();}catch(b){c();}catch(d){}finally{e();}',
+            "try {\n    a();\n}\ncatch (b) {\n    c();\n}\ncatch (d) {}\nfinally {\n    e();\n}");
+        bt('if(a){b();}else if(c) foo();',
+            "if (a) {\n    b();\n}\nelse if (c) foo();");
+        bt("if (a) {\n// comment\n}else{\n// comment\n}",
+            "if (a) {\n    // comment\n}\nelse {\n    // comment\n}"); // if/else statement with empty body
+        bt('if (x) {y} else { if (x) {y}}',
+            'if (x) {\n    y\n}\nelse {\n    if (x) {\n        y\n    }\n}');
+        bt('if (a)\n{\nb;\n}\nelse\n{\nc;\n}',
+            'if (a) {\n    b;\n}\nelse {\n    c;\n}');
+        test_fragment('    /*\n* xx\n*/\n// xx\nif (foo) {\n    bar();\n}',
+                      '    /*\n     * xx\n     */\n    // xx\n    if (foo) {\n        bar();\n    }');
+        bt('if (foo) {}\nelse /regex/.test();');
+        bt('if (foo) /regex/.test();');
         bt('if (a)\n{\nb;\n}\nelse\n{\nc;\n}', 'if (a) {\n    b;\n}\nelse {\n    c;\n}');
+        test_fragment('if (foo) {', 'if (foo) {');
+        test_fragment('foo {', 'foo {');
+        test_fragment('return {', 'return {'); // return needs the brace.
+        test_fragment('return /* inline */ {', 'return /* inline */ {');
+        // test_fragment('return\n{', 'return\n{'); // can't support this?, but that's an improbable and extreme case anyway.
+        test_fragment('return;\n{', 'return; {');
+        bt("throw {}");
+        bt("throw {\n    foo;\n}");
+        bt('var foo = {}');
+        bt('if (foo) bar();\nelse break');
+        bt('function x() {\n    foo();\n}zzz', 'function x() {\n    foo();\n}\nzzz');
+        bt('a: do {} while (); xxx', 'a: do {} while ();\nxxx');
+        bt('var a = new function();');
+        bt('var a = new function() {};');
+        bt('var a = new function a() {};');
+        test_fragment('new function');
 
-        test_fragment('    /*\n* xx\n*/\n// xx\nif (foo) {\n    bar();\n}', '    /*\n     * xx\n     */\n    // xx\n    if (foo) {\n        bar();\n    }');
+        opts.brace_style = 'collapse';
 
-        bt('if (foo) {}\nelse /regex/.test();');
-        bt('if (foo) /regex/.test();');
 
         bt('a = <?= external() ?> ;'); // not the most perfect thing in the world, but you're the weirdo beaufifying php mix-ins with javascript beautifier
         bt('a = <%= external() %> ;');

diff --git a/python/jsbeautifier/__init__.py b/python/jsbeautifier/__init__.py
@@ -250,7 +250,6 @@ def beautify(self, s, opts = None ):
         if opts != None:
             self.opts = opts
 
-
         if self.opts.brace_style not in ['expand', 'collapse', 'end-expand']:
             raise(Exception('opts.brace_style must be "expand", "collapse" or "end-expand".'))
 
@@ -805,15 +804,17 @@ def handle_start_block(self, token_text):
         self.set_mode(MODE.BlockStatement)
 
         empty_braces = self.is_next('}')
-        if self.opts.brace_style == 'expand-strict':
-            if not empty_braces:
+        empty_anonymous_function = empty_braces and self.flags.last_word == 'function' and \
+            self.last_type == 'TK_END_EXPR'
+
+        if self.opts.brace_style == 'expand':
+            if self.last_type != 'TK_OPERATOR' and \
+                (empty_anonymous_function or
+                    self.last_type == 'TK_EQUALS' or
+                    (self.is_special_word(self.flags.last_text) and self.flags.last_text != 'else')):
+                self.output_space_before_token = True
+            else:
                 self.append_newline()
-        elif self.opts.brace_style == 'expand':
-            if self.last_type != 'TK_OPERATOR':
-                if self.last_type == 'TK_EQUALS' or (self.is_special_word(self.flags.last_text) and self.flags.last_text != 'else'):
-                    self.output_space_before_token = True
-                else:
-                    self.append_newline()
         else: # collapse
             if self.last_type not in ['TK_OPERATOR', 'TK_START_EXPR']:
                 if self.last_type == 'TK_START_BLOCK':
@@ -838,12 +839,14 @@ def handle_end_block(self, token_text):
             self.restore_mode()
 
         self.restore_mode()
-        if self.opts.brace_style == 'expand' or self.opts.brace_style == 'expand-strict':
-            if self.last_type != 'TK_START_BLOCK':
+
+        empty_braces = self.last_type == 'TK_START_BLOCK';
+        if self.opts.brace_style == 'expand':
+            if not empty_braces:
                 self.append_newline()
         else:
             # skip {}
-            if self.last_type != 'TK_START_BLOCK':
+            if not empty_braces:
                 if self.is_array(self.flags.mode) and self.opts.keep_array_indentation:
                     self.opts.keep_array_indentation = False
                     self.append_newline()
@@ -938,7 +941,7 @@ def handle_word(self, token_text):
             if token_text not in ['else', 'catch', 'finally']:
                 prefix = 'NEWLINE'
             else:
-                if self.opts.brace_style in ['expand', 'end-expand', 'expand-strict']:
+                if self.opts.brace_style in ['expand', 'end-expand']:
                     prefix = 'NEWLINE'
                 else:
                     prefix = 'SPACE'
@@ -971,8 +974,7 @@ def handle_word(self, token_text):
         if token_text in ['else', 'catch', 'finally']:
             if self.last_type != 'TK_END_BLOCK' \
                or self.opts.brace_style == 'expand' \
-               or self.opts.brace_style == 'end-expand' \
-               or self.opts.brace_style == 'expand-strict':
+               or self.opts.brace_style == 'end-expand':
                 self.append_newline()
             else:
                 self.trim_output(True)