Repository: eslint/doctrine
Pull request #199 (2017-09-18 05:14:59)

fix: Making sure union type stringification respects compact flag
All other types on `stringifyImpl` currently supports the compact flag but `UnionType`. 

Currently regardless of the compact flag Uniontypes are printed as

```
A|B|C
```` 

This PR adds support for printing as:

```
A | B | C 
```

fixes: https://github.com/eslint/doctrine/issues/200

jsf-clabot: [![CLA assistant check](https://cla.js.foundation/pull/badge/not_signed)](https://cla.js.foundation/eslint/doctrine?pullRequest=199) <br/>Thank you for your submission, we really appreciate it. Like many open source projects, we ask that you sign our [Contributor License Agreement](https://cla.js.foundation/eslint/doctrine?pullRequest=199) before we can accept your contribution.<br/>
coveralls: 
[![Coverage Status](https://coveralls.io/builds/13311243/badge)](https://coveralls.io/builds/13311243)

Coverage remained the same at 99.25% when pulling **49f55902af21bd4828d7a33decdbfce2d0ce48e2 on mitermayer:respect-compact-mode** into **e3a011b949338bf5c10978db1b2b71e726cc2a4a on eslint:master**.

coveralls: 
[![Coverage Status](https://coveralls.io/builds/13311243/badge)](https://coveralls.io/builds/13311243)

Coverage remained the same at 99.25% when pulling **49f55902af21bd4828d7a33decdbfce2d0ce48e2 on mitermayer:respect-compact-mode** into **e3a011b949338bf5c10978db1b2b71e726cc2a4a on eslint:master**.

coveralls: 
[![Coverage Status](https://coveralls.io/builds/13311243/badge)](https://coveralls.io/builds/13311243)

Coverage remained the same at 99.25% when pulling **49f55902af21bd4828d7a33decdbfce2d0ce48e2 on mitermayer:respect-compact-mode** into **e3a011b949338bf5c10978db1b2b71e726cc2a4a on eslint:master**.

coveralls: 
[![Coverage Status](https://coveralls.io/builds/13311243/badge)](https://coveralls.io/builds/13311243)

Coverage remained the same at 99.25% when pulling **49f55902af21bd4828d7a33decdbfce2d0ce48e2 on mitermayer:respect-compact-mode** into **e3a011b949338bf5c10978db1b2b71e726cc2a4a on eslint:master**.

mitermayer: anyone can look into this ??? :(
platinumazure: LGTM, just want another set of eyes on this. Thanks for contributing!
mitermayer: @platinumazure any news around this issue ? 

Patch:
diff --git a/lib/doctrine.js b/lib/doctrine.js
index a5804df..bdaa0e0 100644
--- a/lib/doctrine.js
+++ b/lib/doctrine.js
@@ -90,10 +90,6 @@
             title === 'public' || title === 'private' || title === 'protected';
     }
 
-    function trim(str) {
-        return str.replace(/^\s+/, '').replace(/\s+$/, '');
-    }
-
     function unwrapComment(doc) {
         // JSDoc comment is following form
         //   /**
@@ -544,7 +540,7 @@
         };
 
         TagParser.prototype.parseDescription = function parseDescription() {
-            var description = trim(sliceSource(source, index, this._last));
+            var description = sliceSource(source, index, this._last).trim();
             if (description) {
                 if ((/^-\s+/).test(description)) {
                     description = description.substring(2);
@@ -555,15 +551,15 @@
         };
 
         TagParser.prototype.parseCaption = function parseDescription() {
-            var description = trim(sliceSource(source, index, this._last));
+            var description = sliceSource(source, index, this._last).trim();
             var captionStartTag = '<caption>';
             var captionEndTag = '</caption>';
             var captionStart = description.indexOf(captionStartTag);
             var captionEnd = description.indexOf(captionEndTag);
             if (captionStart >= 0 && captionEnd >= 0) {
-                this._tag.caption = trim(description.substring(
-                    captionStart + captionStartTag.length, captionEnd));
-                this._tag.description = trim(description.substring(captionEnd + captionEndTag.length));
+                this._tag.caption = description.substring(
+                    captionStart + captionStartTag.length, captionEnd).trim();
+                this._tag.description = description.substring(captionEnd + captionEndTag.length).trim();
             } else {
                 this._tag.description = description;
             }
@@ -585,7 +581,7 @@
                 'namespace': true,
                 'typedef': true
             };
-            kind = trim(sliceSource(source, index, this._last));
+            kind = sliceSource(source, index, this._last).trim();
             this._tag.kind = kind;
             if (!hasOwnProperty(kinds, kind)) {
                 if (!this.addError('Invalid kind name \'%0\'', kind)) {
@@ -597,7 +593,7 @@
 
         TagParser.prototype.parseAccess = function parseAccess() {
             var access;
-            access = trim(sliceSource(source, index, this._last));
+            access = sliceSource(source, index, this._last).trim();
             this._tag.access = access;
             if (access !== 'private' && access !== 'protected' && access !== 'public') {
                 if (!this.addError('Invalid access name \'%0\'', access)) {
@@ -610,7 +606,7 @@
         TagParser.prototype.parseThis = function parseThis() {
             // this name may be a name expression (e.g. {foo.bar}),
             // an union (e.g. {foo.bar|foo.baz}) or a name path (e.g. foo.bar)
-            var value = trim(sliceSource(source, index, this._last));
+            var value = sliceSource(source, index, this._last).trim();
             if (value && value.charAt(0) === '{') {
                 var gotType = this.parseType();
                 if (gotType && this._tag.type.type === 'NameExpression' || this._tag.type.type === 'UnionType') {
@@ -626,7 +622,7 @@
 
         TagParser.prototype.parseVariation = function parseVariation() {
             var variation, text;
-            text = trim(sliceSource(source, index, this._last));
+            text = sliceSource(source, index, this._last).trim();
             variation = parseFloat(text, 10);
             this._tag.variation = variation;
             if (isNaN(variation)) {
@@ -638,7 +634,7 @@
         };
 
         TagParser.prototype.ensureEnd = function () {
-            var shouldBeEmpty = trim(sliceSource(source, index, this._last));
+            var shouldBeEmpty = sliceSource(source, index, this._last).trim();
             if (shouldBeEmpty) {
                 if (!this.addError('Unknown content \'%0\'', shouldBeEmpty)) {
                     return false;
@@ -819,7 +815,7 @@
                 description += advance();
             }
 
-            return preserveWhitespace ? description : trim(description);
+            return preserveWhitespace ? description : description.trim();
         }
 
         function parse(comment, options) {
diff --git a/lib/typed.js b/lib/typed.js
index f0d759b..e5b14f7 100644
--- a/lib/typed.js
+++ b/lib/typed.js
@@ -1133,7 +1133,7 @@
             for (i = 0, iz = node.elements.length; i < iz; ++i) {
                 result += stringifyImpl(node.elements[i], compact);
                 if ((i + 1) !== iz) {
-                    result += '|';
+                    result += compact ? '|' : ' | ';
                 }
             }
 


Test patch:
diff --git a/test/stringify.js b/test/stringify.js
index f59bb2b..4d2824f 100644
--- a/test/stringify.js
+++ b/test/stringify.js
@@ -237,7 +237,7 @@ describe('Expression', function () {
                 type: doctrine.Syntax.NameExpression,
                 name: 'Number'
             }]
-        }).should.equal('(String|Number)');
+        }, {compact: false }).should.equal('(String | Number)');
 
         doctrine.type.stringify({
             type: doctrine.Syntax.UnionType,
@@ -248,7 +248,7 @@ describe('Expression', function () {
                 type: doctrine.Syntax.NameExpression,
                 name: 'Number'
             }]
-        }, { topLevel: true }).should.equal('String|Number');
+        }, { topLevel: true, compact: true }).should.equal('String|Number');
     });
 
     it('RestType', function () {
@@ -382,7 +382,7 @@ describe('Complex identity', function () {
             doctrine.type.parseType(data07)
         ).should.equal(data07);
 
-        var data08 = 'function (new: Date, a: Array.<String, Number>, b: (Number|String|Date)): HashMap.<String, Number>';
+        var data08 = 'function (new: Date, a: Array.<String, Number>, b: (Number | String | Date)): HashMap.<String, Number>';
         doctrine.type.stringify(
             doctrine.type.parseType(data08)
         ).should.equal(data08);