Skip to content

valid-types: Add more type checks #308

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged

Conversation

l1bbcsg
Copy link
Contributor

@l1bbcsg l1bbcsg commented Jul 4, 2019

Fixes #305 .

All of the problems mentioned in the issue are solved, but I'm not entirely sure the solution's perfect.

Namely, there's a lot of complexity because there are tags that may have a name, a type or a description and tags must have a name, a type or a description and each of this sets intersect with every other.

The changes introduced in this PR:

  • The rule became stricter as it now checks more and reports more errors as a result. Some previously overlooked errors may now be reported as well.
  • Some error messages changed, namely Syntax error in type: (note the empty string) now became more specific Tag @tag must have a type or Tag @tag must have a namepath
  • Parsing of @see tag has changed. Previously @see {@link http://foo.bar} was parsed as having type @link http://foo.bar which definitely not something that should happen. I made {@link http://foo.bar} a description.

The following became unintentionally valid:

/**
 * @typedef {number|string}
 */

I think it's fine, at least as far as this rule is concerned. The type is valid, so is jsdoc itself.
The fact that it's missing typedef name via variable declaration could probably be handled by another rule. This was done to allow GCC syntax.

Also, this is valid with previous and current checks, but shouldn't be (Array<string> is not a valid identifier). This happens because both type and namepath are checked the same way via jsdoctypeparser.

/**
 * @typedef {StringArray} Array<string>
 */

Copy link
Collaborator

@brettz9 brettz9 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some useful contributions, thank you. There are a number of questions/concerns, in part due to jsdoc not being a strict validation spec.

l1bbcsg and others added 26 commits July 5, 2019 16:34
* master:
  feat(require-description-complete-sentence): validate explicit `description` tags
  fix(check-types): remove template substitution variables, `badType`, `preferredType`, and `replacement` (gajus#310)
  feat: remove `preferredTypesDefined` option in favor of making it apply automatically
  feat(check-tag-names): auto-allow tags in `tagNamePreference` setting to be defined; add `definedTags` option to replace `additionalTagNames.customTags` setting
  feat(check-tag-names): allow rejecting normally valid tag names and/or providing custom error messages when suggesting tags to reject or replace (fixes gajus#108)
  Allow rejecting normally valid tag names and/or providing custom error messages when suggesting tags to reject or replace (gajus#302)
  docs: indicate option defaults (fixes part of gajus#256) and ensure optioins all listed in docs
  chore: update Babel devDeps.
  feat(no-undefined-types): handle GCC generic functions
  docs: generate docs
  Add an invalid test case for incorrect @template type
  fix: reverse jsdoctypeparser update until traversal issue with "external:..." can be fixed
  feat: bump jsdoctypeparser to 5.0.0
  docs: generate docs
  Add support for generic functions in addition to classes
* master:
  feat(match-description): allow `description` and `desc` tags to be matched for validation
  refactor: use template string when appropriate
* master:
  docs: correct docs
  refactor: change option extraction codes
  refactor: leverage `settings` property from `iterateJsdoc.js` for consistent settings access
  docs: update docs
  feat(`require-example`): move settings to options
  feat(`require-jsdoc`): move settings to options
  feat(`valid-types`): move settings to options
  feat(`require-returns`): move settings to options
  fix(`check-examples`): make `escape-regex-string` a dependency, as it should have been (fixes gajus#314)

# Conflicts:
#	README.md
#	src/rules/validTypes.js
#	test/rules/assertions/validTypes.js
* master:
  refactor: fix code comment
  docs(require-description, require-example, require-param, require-returns): add reference to potentially applicable settings, removing reference to deprecated items
  docs: make explicit default for setting `ignorePrivate` and clarify setting
…-type-checks-to-valid-types

* commit 'f44b25ed4cb521116a682a0d8d10406f3440cec7':
  refactor: simplify iterateJsdoc
* master:
  feat(`require-param`): remove deprecated settings `allowOverrideWithoutParam`, `allowImplementsWithoutParam`, `allowAugmentsExtendsWithoutParam`
  feat(`check-examples`): move settings to options (fixes gajus#216)
* master:
  feat(`require-description`): add `descriptionStyle` option to accept implicit descriptions as satisfying the rule by default (fixes gajus#301)
  feat(`match-description`): allow arbitrary tags; also fixes `returns`/`return` to avoid skipping matching of an initial "name" and space
* master: (46 commits)
  feat(require-description-complete-sentence): limit checking to certain default tags likely to have descriptions or by `tags` array for additional choices; fixes gajus#337
  docs(newline-after-description): indicate applies on doc block
  docs(match-description): add alias `desc` to separate column
  docs(match-description): indicate application by default to `description`/`desc` and allowance for `property`/`prop`; clarify
  fix(match-description): ensure `prop` and `property` matching excludes name
  testing(require-param): fix test source (part of gajus#332)
  testing(require-param): fix test expectation (part of gajus#332)
  docs: generate docs
  chore: update devDeps (eslint-config-canonical, gitdown)
  fix(no-undefined-types): ensure working in all contexts; fixes gajus#324
  refactor(iterateJsdoc): reduce retrieval calls
  docs(check-examples): allow for whitespace at end
  feat(check-examples): add `paddedIndent` option
  fix(check-examples): preserve whitespace so as to report issues with whitespace-related rules such as `indent` (fixes gajus#211)
  chore(travis): fix Travis to use older unicorn version that supports Node 6 (and the canonical config that requires the earlier unicorn version)
  docs(require-returns, require-returns-check): indicate that these will report if multiple `returns` tags are present
  refactor: use `String.prototype.repeat` over lodash `repeat`
  refactor: apply (jsdoc-related) eslint rule fixes
  chore: add `lint-fix` script
  fix(newline-after-description): avoid matching duplicate or partial matches within tag descriptions after the block description; fixes gajus#328
  ...
* master:
  fix(match-description, require-description-complete-sentence): do not report blocking of tag by user for `description` tag (the tag is not required by these rules)
  fix(require-description): only report blocking of description tag by user if "tag" option is set
  fix(require-description, implements-on-classes, check-param-names, check-tag-names, require-param, require-returns, require-returns-check, require-hyphen-before-param-description, require-param-description, require-param-name, require-param-type, require-returns-description, require-returns-type): only report blocking of tag by user if problematic tag is present; fixes gajus#332
* master:
  chore: change the process for generatng readme
  fix: Fix wrong alignment
  docs: generate docs
  fix: Exclude line slicing by zero.
  chore: update comment-parser and gitdown deps.
  fix: Fix trim bug in trimCode
  docs: generate docs
  fix: Remove unformatted space
* master:
  feat: add fixer for no-types (gajus#350)
  fix(check-param-names): ensure polyfilling `Object.entries` for Node 6
  fix(check-param-names): provide fixer duplicate param names (remove extra); partial fix for gajus#47
  chore(npm): update deps (comment-parser, lodash) and devDep (husky, mocha)
  fix(require-param): reporting all missing jsdoc params (gajus#348)
  docs(check-examples): fix heading level
  docs(readme): remove section "Reference to jscs-jsdoc"
  chore: update Babel devDeps
…o nothing with missing description); fixes part of gajus#336
feat(*): have doc block retrieval allow or require additional preceding empty lines via min/max line setting API
@l1bbcsg
Copy link
Contributor Author

l1bbcsg commented Aug 8, 2019

Apologizes for dying on this PR, both work and vacation happened.

I updated it and addressed all comments I believe.

* master:
  chore: report unused disable directives in eslint
  refactor: remove unused disable directives
  fix(require-jsdoc): avoid exported functions possessing jsdoc blocks causing other non-public functions to be treated as exported; fixes gajus#358
  refactor(require-jsdoc): remove redundant check
  docs(require-returns): Fix typo
  chore: update devDeps; avoid `lint-staged`
  Add type "symbol" to noUndefinedTypes
  chore: update `lint-staged` for husky issue: typicode/husky#247
  chore(CI): stop running broken npm script `check-readme`
  fix(no-undefined-types): avoid `flat-map-polyfill` entirely; further further fix for gajus#366
  fix(check-tag-names): ensure `replacement` field overrides a default tag name preference; fixes gajus#367
  chore: update comment-parser dep. and devDeps; avoid buggy eslint6.2 and for eslint 6 tests, avoid breaking typescript-eslint/parser update
  fix(`no-undefined-types`): import flat-map polyfill directly to avoid renamed `flatten` method from polluting prototype (gajus#366)
@brettz9 brettz9 mentioned this pull request Sep 1, 2019
13 tasks
Copy link
Collaborator

@brettz9 brettz9 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good now (note, I did make a few subsequent minor additions/changes). I've added some of the GCC-specific to-dos to #356.

@gajus
Copy link
Owner

gajus commented Sep 1, 2019

🎉 This PR is included in version 15.9.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

valid-types: Only checks the name of typedef but not the definition
5 participants