jscs-dev / jscs-jsdoc

JsDoc validation rules for jscs
MIT License
99 stars 35 forks source link

checkReturnTypes fails for @typedef #32

Closed cowwoc closed 9 years ago

cowwoc commented 10 years ago

Given:

        /**
         * @typedef {Object} UsersState
         *
         * @property {String} id the DOM id of the component that generated the state
         * @property {Object} state the page's state
         */

        /**
         * @return {UsersState} the page's state
         */
        Users.prototype.getState = function()
        {
            return {
                id: "main",
                state:
                {
                    table: this.table.getState(),
                    addUserModal: this.addUserModal.getState(),
                    editUserModal: this.editUserModal.getState()
                }
            };
        };

The function returns the correct type (following duck typing) yet jscs-jsdoc fails with:

Wrong returns value at ../../../target/classes/js/html/Users.js :
   322 |  Users.prototype.getState = function()
   323 |  {
   324 |   return {
------------------^
   325 |    id: "main",
   326 |    state:

I believe my syntax is correct because http://justjson.blogspot.ca/2013/07/jsdoc-documenting-configparamcustom.html does the same for the @returns {JustJson.Greeting.Config} example.

qfox commented 10 years ago

Thanks for contributing.

I think it's a bug in matching logic. Need to add this as a test case and fix.

qfox commented 10 years ago

Almost done. I'll merge branch into master soon.

cowwoc commented 9 years ago

@zxqfox That last commit did not actually fix my issue. The test does not correspond to what I am actually doing. The test returns a non-existent type called {Class} whereas I'm returning a @typedef called {UsersState}. Can you please reopen this issue?

qfox commented 9 years ago

@cowwoc Did you upgraded to v0.0.20?

qfox commented 9 years ago

@cowwoc @typedef shouldn't care about another tags. Btw now rule checkTypes forces testing existent types for bunch of tags (@typedef, @property, etc.).

Also I've added a test case for this issue and it works fine. ;-\

cowwoc commented 9 years ago

@zxqfox Yes, I updated to 0.0.20 and I am still seeing this problem. I will try to provide you with more information in a minute.

cowwoc commented 9 years ago

@typedef shouldn't care about another tags.

What does this mean?

Okay, now here is how I am able to reproduce this issue:

Create Aaa.js with the following contents:

/* global define:false */

define(function()
{
    "use strict";
    var Test = {};
    /**
     * @typedef {Object} AuthenticationsState
     *
     * @property {String} id the DOM id of the component that generated the state
     * @property {Object} state the page's state
     */

    /**
     * @return {AuthenticationsState} the page's state
     */
    Test.prototype.getState = function()
    {
        return {
            id: "main",
            state:
                {
                    loginForm: this.loginForm.getState(),
                    signupForm: this.signupForm.getState()
                }
        };
    };

    return Test;
});

Here is my package.json file:

{
    "name": "myproject",
    "private": true,
    "dependencies":
        {
            "grunt": "~0.4.5",
            "grunt-cli": "~0.1.13",
            "grunt-contrib-jshint": "~0.10.0",
            "grunt-contrib-csslint": "~0.3.1",
            "grunt-jscs": "~0.8.1",
            "jscs": "~1.7.3",
            "jscs-jsdoc": "~0.0.20",
            "csslint": "~0.10.0",
            "jshint": "~2.5.6"
        }
}

Here is my Gruntfile.js:

module.exports = function(grunt)
{
    grunt.initConfig(
        {
            pkg: grunt.file.readJSON("package.json"),
            csslint:
                {
                    src: "../../../target/classes/css/**/*.css",
                    options:
                        {
                            absoluteFilePathsForFormatters: true,
                            important: 2,
                            "adjoining-classes": false,
                            "known-properties": 2,
                            "box-sizing": false,
                            // WORKAROUND: https://github.com/CSSLint/csslint/issues/544
                            "box-model": false,
                            "overqualified-elements": true,
                            "display-property-grouping": 2,
                            "bulletproof-font-face": false,
                            "compatible-vendor-prefixes": true,
                            "regex-selectors": false,
                            "errors": 2,
                            "duplicate-background-images": true,
                            "duplicate-properties": 2,
                            "empty-rules": true,
                            "selector-max-approaching": true,
                            gradients: true,
                            "fallback-colors": false,
                            "font-sizes": true,
                            "font-faces": 2,
                            "floats": true,
                            "star-property-hack": 2,
                            "outline-none": true,
                            "import": false,
                            "ids": false,
                            "underscore-property-hack": 2,
                            "rules-count": 2,
                            "qualified-headings": 2,
                            "selector-max": 2,
                            shorthand: false,
                            "text-indent": 2,
                            "unique-headings": 2,
                            "universal-selector": true,
                            "unqualified-attributes": true,
                            "vendor-prefix": 2,
                            "zero-units": 2
                        }
                },
            jshint:
                {
                    all: ["../../../target/classes/js/**/*.js"],
                    options:
                        {
                            bitwise: true,
                            eqeqeq: true,
                            forin: true,
                            freeze: true,
                            latedef: true,
                            noarg: true,
                            nonbsp: true,
                            nonew: true,
                            undef: true,
                            unused: "vars",
                            strict: true,
                            maxdepth: 4,
                            browser: true,
                            jquery: true,
                            devel: true
                        }
                },
            jscs:
                {
                    src: "../../../target/classes/js/**/*.js",
                    options:
                        {
                            requireCurlyBraces: ["try", "catch"],
                            requireSpaceAfterKeywords: ["if", "for", "while", "do", "switch", "return", "catch"],
                            requireSpacesInConditionalExpression: true,
                            requireSpaceBeforeBlockStatements: true,
                            requireSpacesInFunctionExpression:
                                {
                                    beforeOpeningCurlyBrace: true
                                },
                            disallowSpacesInFunctionExpression:
                                {
                                    beforeOpeningRoundBrace: true
                                },
                            requireSpacesInFunctionDeclaration:
                                {
                                    beforeOpeningCurlyBrace: true
                                },
                            disallowSpacesInFunctionDeclaration:
                                {
                                    beforeOpeningRoundBrace: true
                                },
                            // WORKAROUND: https://netbeans.org/bugzilla/show_bug.cgi?id=248443
//                          disallowSpacesInCallExpression: true,
                            requireBlocksOnNewline: true,
                            disallowPaddingNewlinesInBlocks: true,
                            requirePaddingNewLinesInObjects: true,
                            disallowEmptyBlocks: true,
                            disallowQuotedKeysInObjects: true,
                            disallowDanglingUnderscores: true,
                            requireCommaBeforeLineBreak: true,
                            requireOperatorBeforeLineBreak: ["?", "+", "-", "/", "*", "=", "==", "===", "!=",
                                "!==", ">", ">=", "<", "<="],
                            disallowSpaceAfterPrefixUnaryOperators: ["++", "--", "+", "-", "~", "!"],
                            disallowSpaceBeforePostfixUnaryOperators: ["++", "--"],
                            disallowSpaceBeforeBinaryOperators: [","],
                            requireSpaceBeforeBinaryOperators: ["+", "-", "/", "*", "=", "==", "===", "!=", "!==",
                                ">", ">=", "<", "<="],
                            requireSpaceAfterBinaryOperators: ["+", "-", "/", "*", "=", "==", "===", "!=", "!=="],
                            requireCamelCaseOrUpperCaseIdentifiers: true,
                            disallowMultipleLineStrings: true,
                            disallowMultipleLineBreaks: true,
                            validateQuoteMarks:
                                {
                                    "mark": "\"", "escape": true
                                },
// WORKAROUND: https://netbeans.org/bugzilla/show_bug.cgi?id=227007
//validateIndentation: "\t",
                            disallowMixedSpacesAndTabs: "smart",
                            disallowTrailingWhitespace: true,
                            disallowTrailingComma: true,
                            requireKeywordsOnNewLine: ["else"],
                            maximumLineLength:
                                {
                                    value: 120,
                                    tabSize: 2,
                                    allowUrlComments: true
                                },
                            requireCapitalizedConstructors: true,
                            disallowYodaConditions: true,
                            validateParameterSeparator: ", ",
                            requireNewlineBeforeBlockStatements: true,
                            maxErrors: 10,
                            additionalRules:
                                [
                                    "node_modules/jscs-jsdoc/lib/rules/*.js"
                                ],
                            jsDoc:
                                {
                                    // WORKAROUND: https://github.com/jscs-dev/jscs-jsdoc/issues/33
//                                  checkParamNames: true,
                                    requireParamTypes: true,
                                    checkRedundantParams: true,
                                    // WORKAROUND: https://github.com/jscs-dev/jscs-jsdoc/issues/32
                                    // WORKAROUND: https://github.com/jscs-dev/jscs-jsdoc/issues/34
                                    checkReturnTypes: true,
                                    // WORKAROUND: https://github.com/jscs-dev/jscs-jsdoc/issues/35
//                                  checkTypes: true,
                                    // WORKAROUND: https://github.com/jscs-dev/jscs-jsdoc/issues/31
//                                  checkRedundantReturns: true,
                                    checkRedundantAccess: true,
                                    leadingUnderscoreAccess: "private"
//                                  enforceExistence: true
                                }
                        }
                }
        });
    grunt.loadNpmTasks("grunt-contrib-csslint");
    grunt.loadNpmTasks("grunt-contrib-jshint");
    grunt.loadNpmTasks("grunt-jscs");
    grunt.registerTask("build", ["csslint", "jshint", "jscs"]);
};

I get this error:

Wrong returns value at ../../../target/classes/js/html/Aaa.js :
    17 | Test.prototype.getState = function()
    18 | {
    19 |  return {
-----------------^
    20 |   id: "main",
    21 |   state:

I think most of this configuration is meaningless (can be trimmed) but I am including it for the sake of completion.

qfox commented 9 years ago

Found a bug. ;-( Fixing... Thanks!

qfox commented 9 years ago

Merged into v0.0.21

@cowwoc Please check this

cowwoc commented 9 years ago

@zxqfox Works for me, thanks!