From b12cff891af70269c0547127b1724494466ddd98 Mon Sep 17 00:00:00 2001
From: Orlando Wenzinger <orlando.wenzinger@bluewin.ch>
Date: Sun, 22 Oct 2017 12:58:38 -0700
Subject: [PATCH] Fix: Expected order of jsdoc tags (fixes #9412) (#9451)

* Chore: Add tests to validate the jsdoc tag order (refs #9412)

* Fix: Expected order of jsdoc tags (fixes #9412)

* Chore: Improve code readability (refs #9412)

Create spelling variable names and remove unecessary if statement.
---
 lib/rules/valid-jsdoc.js       |  72 +++---
 tests/lib/rules/valid-jsdoc.js | 428 +++++++++++++++++++++++++++++++++
 2 files changed, 467 insertions(+), 33 deletions(-)

diff --git a/lib/rules/valid-jsdoc.js b/lib/rules/valid-jsdoc.js
index d4ee4e07382..ea60b115ce6 100644
--- a/lib/rules/valid-jsdoc.js
+++ b/lib/rules/valid-jsdoc.js
@@ -226,8 +226,10 @@ module.exports = {
         function checkJSDoc(node) {
             const jsdocNode = sourceCode.getJSDocComment(node),
                 functionData = fns.pop(),
-                params = Object.create(null);
+                params = Object.create(null),
+                paramsTags = [];
             let hasReturns = false,
+                returnsTag,
                 hasConstructor = false,
                 isInterface = false,
                 isOverride = false,
@@ -261,43 +263,13 @@ module.exports = {
                         case "param":
                         case "arg":
                         case "argument":
-                            if (!tag.type) {
-                                context.report({ node: jsdocNode, message: "Missing JSDoc parameter type for '{{name}}'.", data: { name: tag.name } });
-                            }
-
-                            if (!tag.description && requireParamDescription) {
-                                context.report({ node: jsdocNode, message: "Missing JSDoc parameter description for '{{name}}'.", data: { name: tag.name } });
-                            }
-
-                            if (params[tag.name]) {
-                                context.report({ node: jsdocNode, message: "Duplicate JSDoc parameter '{{name}}'.", data: { name: tag.name } });
-                            } else if (tag.name.indexOf(".") === -1) {
-                                params[tag.name] = 1;
-                            }
+                            paramsTags.push(tag);
                             break;
 
                         case "return":
                         case "returns":
                             hasReturns = true;
-
-                            if (!requireReturn && !functionData.returnPresent && (tag.type === null || !isValidReturnType(tag)) && !isAbstract) {
-                                context.report({
-                                    node: jsdocNode,
-                                    message: "Unexpected @{{title}} tag; function has no return statement.",
-                                    data: {
-                                        title: tag.title
-                                    }
-                                });
-                            } else {
-                                if (requireReturnType && !tag.type) {
-                                    context.report({ node: jsdocNode, message: "Missing JSDoc return type." });
-                                }
-
-                                if (!isValidReturnType(tag) && !tag.description && requireReturnDescription) {
-                                    context.report({ node: jsdocNode, message: "Missing JSDoc return description." });
-                                }
-                            }
-
+                            returnsTag = tag;
                             break;
 
                         case "constructor":
@@ -333,6 +305,40 @@ module.exports = {
                     }
                 });
 
+                paramsTags.forEach(param => {
+                    if (!param.type) {
+                        context.report({ node: jsdocNode, message: "Missing JSDoc parameter type for '{{name}}'.", data: { name: param.name } });
+                    }
+                    if (!param.description && requireParamDescription) {
+                        context.report({ node: jsdocNode, message: "Missing JSDoc parameter description for '{{name}}'.", data: { name: param.name } });
+                    }
+                    if (params[param.name]) {
+                        context.report({ node: jsdocNode, message: "Duplicate JSDoc parameter '{{name}}'.", data: { name: param.name } });
+                    } else if (param.name.indexOf(".") === -1) {
+                        params[param.name] = 1;
+                    }
+                });
+
+                if (hasReturns) {
+                    if (!requireReturn && !functionData.returnPresent && (returnsTag.type === null || !isValidReturnType(returnsTag)) && !isAbstract) {
+                        context.report({
+                            node: jsdocNode,
+                            message: "Unexpected @{{title}} tag; function has no return statement.",
+                            data: {
+                                title: returnsTag.title
+                            }
+                        });
+                    } else {
+                        if (requireReturnType && !returnsTag.type) {
+                            context.report({ node: jsdocNode, message: "Missing JSDoc return type." });
+                        }
+
+                        if (!isValidReturnType(returnsTag) && !returnsTag.description && requireReturnDescription) {
+                            context.report({ node: jsdocNode, message: "Missing JSDoc return description." });
+                        }
+                    }
+                }
+
                 // check for functions missing @returns
                 if (!isOverride && !hasReturns && !hasConstructor && !isInterface &&
                     node.parent.kind !== "get" && node.parent.kind !== "constructor" &&
diff --git a/tests/lib/rules/valid-jsdoc.js b/tests/lib/rules/valid-jsdoc.js
index e9519278a48..54fea8d8fdb 100644
--- a/tests/lib/rules/valid-jsdoc.js
+++ b/tests/lib/rules/valid-jsdoc.js
@@ -499,6 +499,434 @@ ruleTester.run("valid-jsdoc", rule, {
             "function foo(){ throw new Error('Not Implemented'); }",
             options: [{ requireReturn: false }]
         },
+
+        // https://github.com/eslint/eslint/issues/9412 - different orders for jsodc tags
+        {
+            code:
+            "/**\n" +
+            "* Description\n" +
+            "* @return {Number} desc\n" +
+            "* @constructor \n" +
+            "* @override\n" +
+            "* @abstract\n" +
+            "* @interface\n" +
+            "* @param {string} hi - desc\n" +
+            "*/\n" +
+            "function foo(hi){ return 1; }",
+            options: [{ requireReturn: false }]
+        },
+        {
+            code:
+            "/**\n" +
+            "* Description\n" +
+            "* @returns {Number} desc\n" +
+            "* @class \n" +
+            "* @inheritdoc\n" +
+            "* @virtual\n" +
+            "* @interface\n" +
+            "* @param {string} hi - desc\n" +
+            "*/\n" +
+            "function foo(hi){ return 1; }",
+            options: [{ requireReturn: false }]
+        },
+        {
+            code:
+            "/**\n" +
+            "* Description\n" +
+            "* @return {Number} desc\n" +
+            "* @constructor \n" +
+            "* @override\n" +
+            "* @abstract\n" +
+            "* @interface\n" +
+            "* @arg {string} hi - desc\n" +
+            "*/\n" +
+            "function foo(hi){ return 1; }",
+            options: [{ requireReturn: false }]
+        },
+        {
+            code:
+            "/**\n" +
+            "* Description\n" +
+            "* @returns {Number} desc\n" +
+            "* @class \n" +
+            "* @inheritdoc\n" +
+            "* @virtual\n" +
+            "* @interface\n" +
+            "* @arg {string} hi - desc\n" +
+            "*/\n" +
+            "function foo(hi){ return 1; }",
+            options: [{ requireReturn: false }]
+        },
+        {
+            code:
+            "/**\n" +
+            "* Description\n" +
+            "* @return {Number} desc\n" +
+            "* @constructor \n" +
+            "* @override\n" +
+            "* @abstract\n" +
+            "* @interface\n" +
+            "* @argument {string} hi - desc\n" +
+            "*/\n" +
+            "function foo(hi){ return 1; }",
+            options: [{ requireReturn: false }]
+        },
+        {
+            code:
+            "/**\n" +
+            "* Description\n" +
+            "* @returns {Number} desc\n" +
+            "* @class \n" +
+            "* @inheritdoc\n" +
+            "* @virtual\n" +
+            "* @interface\n" +
+            "* @argument {string} hi - desc\n" +
+            "*/\n" +
+            "function foo(hi){ return 1; }",
+            options: [{ requireReturn: false }]
+        },
+        {
+            code:
+            "/**\n" +
+            "* Description\n" +
+            "* @constructor \n" +
+            "* @override\n" +
+            "* @abstract\n" +
+            "* @interface\n" +
+            "* @param {string} hi - desc\n" +
+            "* @return {Number} desc\n" +
+            "*/\n" +
+            "function foo(hi){ return 1; }",
+            options: [{ requireReturn: false }]
+        },
+        {
+            code:
+            "/**\n" +
+            "* Description\n" +
+            "* @class \n" +
+            "* @inheritdoc\n" +
+            "* @virtual\n" +
+            "* @interface\n" +
+            "* @arg {string} hi - desc\n" +
+            "* @return {Number} desc\n" +
+            "*/\n" +
+            "function foo(hi){ return 1; }",
+            options: [{ requireReturn: false }]
+        },
+        {
+            code:
+            "/**\n" +
+            "* Description\n" +
+            "* @argument {string} hi - desc\n" +
+            "* @return {Number} desc\n" +
+            "*/\n" +
+            "function foo(hi){ return 1; }",
+            options: [{ requireReturn: false }]
+        },
+        {
+            code:
+            "/**\n" +
+            "* Description\n" +
+            "* @constructor \n" +
+            "* @override\n" +
+            "* @abstract\n" +
+            "* @interface\n" +
+            "* @param {string} hi - desc\n" +
+            "* @returns {Number} desc\n" +
+            "*/\n" +
+            "function foo(hi){ return 1; }",
+            options: [{ requireReturn: false }]
+        },
+        {
+            code:
+            "/**\n" +
+            "* Description\n" +
+            "* @class \n" +
+            "* @inheritdoc\n" +
+            "* @virtual\n" +
+            "* @interface\n" +
+            "* @arg {string} hi - desc\n" +
+            "* @returns {Number} desc\n" +
+            "*/\n" +
+            "function foo(hi){ return 1; }",
+            options: [{ requireReturn: false }]
+        },
+        {
+            code:
+            "/**\n" +
+            "* Description\n" +
+            "* @argument {string} hi - desc\n" +
+            "* @returns {Number} desc\n" +
+            "*/\n" +
+            "function foo(hi){ return 1; }",
+            options: [{ requireReturn: false }]
+        },
+        {
+            code:
+            "/**\n" +
+            "* Description\n" +
+            "* @override\n" +
+            "* @abstract\n" +
+            "* @interface\n" +
+            "* @param {string} hi - desc\n" +
+            "* @return {Number} desc\n" +
+            "* @constructor\n" +
+            "*/\n" +
+            "function foo(hi){ return 1; }",
+            options: [{ requireReturn: false }]
+        },
+        {
+            code:
+            "/**\n" +
+            "* Description\n" +
+            "* @inheritdoc\n" +
+            "* @virtual\n" +
+            "* @interface\n" +
+            "* @arg {string} hi - desc\n" +
+            "* @returns {Number} desc\n" +
+            "* @constructor\n" +
+            "*/\n" +
+            "function foo(hi){ return 1; }",
+            options: [{ requireReturn: false }]
+        },
+        {
+            code:
+            "/**\n" +
+            "* Description\n" +
+            "* @argument {string} hi - desc\n" +
+            "* @constructor\n" +
+            "*/\n" +
+            "function foo(hi){}",
+            options: [{ requireReturn: false }]
+        },
+        {
+            code:
+            "/**\n" +
+            "* Description\n" +
+            "* @override\n" +
+            "* @abstract\n" +
+            "* @interface\n" +
+            "* @param {string} hi - desc\n" +
+            "* @return {Number} desc\n" +
+            "* @class\n" +
+            "*/\n" +
+            "function foo(hi){ return 1; }",
+            options: [{ requireReturn: false }]
+        },
+        {
+            code:
+            "/**\n" +
+            "* Description\n" +
+            "* @inheritdoc\n" +
+            "* @virtual\n" +
+            "* @interface\n" +
+            "* @arg {string} hi - desc\n" +
+            "* @returns {Number} desc\n" +
+            "* @class\n" +
+            "*/\n" +
+            "function foo(hi){ return 1; }",
+            options: [{ requireReturn: false }]
+        },
+        {
+            code:
+            "/**\n" +
+            "* Description\n" +
+            "* @argument {string} hi - desc\n" +
+            "* @class \n" +
+            "*/\n" +
+            "function foo(hi){}",
+            options: [{ requireReturn: false }]
+        },
+        {
+            code:
+            "/**\n" +
+            "* Description\n" +
+            "* @abstract\n" +
+            "* @interface\n" +
+            "* @param {string} hi - desc\n" +
+            "* @return {Number} desc\n" +
+            "* @constructor\n" +
+            "* @override\n" +
+            "*/\n" +
+            "function foo(hi){ return 1; }",
+            options: [{ requireReturn: false }]
+        },
+        {
+            code:
+            "/**\n" +
+            "* Description\n" +
+            "* @virtual\n" +
+            "* @interface\n" +
+            "* @arg {string} hi - desc\n" +
+            "* @returns {Number} desc\n" +
+            "* @class\n" +
+            "* @override\n" +
+            "*/\n" +
+            "function foo(hi){ return 1; }",
+            options: [{ requireReturn: false }]
+        },
+        {
+            code:
+            "/**\n" +
+            "* Description\n" +
+            "* @argument {string} hi - desc\n" +
+            "* @override\n" +
+            "*/\n" +
+            "function foo(hi){}",
+            options: [{ requireReturn: false }]
+        },
+        {
+            code:
+            "/**\n" +
+            "* Description\n" +
+            "* @abstract\n" +
+            "* @interface\n" +
+            "* @param {string} hi - desc\n" +
+            "* @return {Number} desc\n" +
+            "* @constructor\n" +
+            "* @inheritdoc\n" +
+            "*/\n" +
+            "function foo(hi){ return 1; }",
+            options: [{ requireReturn: false }]
+        },
+        {
+            code:
+            "/**\n" +
+            "* Description\n" +
+            "* @virtual\n" +
+            "* @interface\n" +
+            "* @arg {string} hi - desc\n" +
+            "* @returns {Number} desc\n" +
+            "* @class\n" +
+            "* @inheritdoc\n" +
+            "*/\n" +
+            "function foo(hi){ return 1; }",
+            options: [{ requireReturn: false }]
+        },
+        {
+            code:
+            "/**\n" +
+            "* Description\n" +
+            "* @argument {string} hi - desc\n" +
+            "* @inheritdoc\n" +
+            "*/\n" +
+            "function foo(hi){}",
+            options: [{ requireReturn: false }]
+        },
+        {
+            code:
+            "/**\n" +
+            "* Description\n" +
+            "* @interface\n" +
+            "* @param {string} hi - desc\n" +
+            "* @return {Number} desc\n" +
+            "* @constructor\n" +
+            "* @override\n" +
+            "* @abstract\n" +
+            "*/\n" +
+            "function foo(hi){ return 1; }",
+            options: [{ requireReturn: false }]
+        },
+        {
+            code:
+            "/**\n" +
+            "* Description\n" +
+            "* @interface\n" +
+            "* @arg {string} hi - desc\n" +
+            "* @returns {Number} desc\n" +
+            "* @class\n" +
+            "* @override\n" +
+            "* @abstract\n" +
+            "*/\n" +
+            "function foo(hi){ return 1; }",
+            options: [{ requireReturn: false }]
+        },
+        {
+            code:
+            "/**\n" +
+            "* Description\n" +
+            "* @argument {string} hi - desc\n" +
+            "* @abstract\n" +
+            "*/\n" +
+            "function foo(hi){}",
+            options: [{ requireReturn: false }]
+        },
+        {
+            code:
+            "/**\n" +
+            "* Description\n" +
+            "* @interface\n" +
+            "* @param {string} hi - desc\n" +
+            "* @return {Number} desc\n" +
+            "* @constructor\n" +
+            "* @override\n" +
+            "* @virtual\n" +
+            "*/\n" +
+            "function foo(hi){ return 1; }",
+            options: [{ requireReturn: false }]
+        },
+        {
+            code:
+            "/**\n" +
+            "* Description\n" +
+            "* @interface\n" +
+            "* @arg {string} hi - desc\n" +
+            "* @returns {Number} desc\n" +
+            "* @class\n" +
+            "* @override\n" +
+            "* @virtual\n" +
+            "*/\n" +
+            "function foo(hi){ return 1; }",
+            options: [{ requireReturn: false }]
+        },
+        {
+            code:
+            "/**\n" +
+            "* Description\n" +
+            "* @argument {string} hi - desc\n" +
+            "* @virtual\n" +
+            "*/\n" +
+            "function foo(hi){}",
+            options: [{ requireReturn: false }]
+        },
+        {
+            code:
+            "/**\n" +
+            "* Description\n" +
+            "* @param {string} hi - desc\n" +
+            "* @return {Number} desc\n" +
+            "* @constructor \n" +
+            "* @override\n" +
+            "* @abstract\n" +
+            "* @interface\n" +
+            "*/\n" +
+            "function foo(hi){ return 1; }",
+            options: [{ requireReturn: false }]
+        },
+        {
+            code:
+            "/**\n" +
+            "* Description\n" +
+            "* @arg {string} hi - desc\n" +
+            "* @returns {Number} desc\n" +
+            "* @class\n" +
+            "* @override\n" +
+            "* @virtual\n" +
+            "* @interface\n" +
+            "*/\n" +
+            "function foo(hi){ return 1; }",
+            options: [{ requireReturn: false }]
+        },
+        {
+            code:
+            "/**\n" +
+            "* Description\n" +
+            "* @argument {string} hi - desc\n" +
+            "* @interface\n" +
+            "*/\n" +
+            "function foo(hi){}",
+            options: [{ requireReturn: false }]
+        },
         {
             code:
             "/**\n" +