Skip to content
This repository has been archived by the owner on May 1, 2019. It is now read-only.

Jsdoc tags #641

Merged
merged 4 commits into from
Apr 27, 2017
Merged

Jsdoc tags #641

merged 4 commits into from
Apr 27, 2017

Conversation

justinfagnani
Copy link
Contributor

Adds support for @customElement, @polymer, @mixinFunction, @mixinClass, and @appliesMixin.

Removes the EditorService which got test times down to ~2s.

Refactors some JSDoc handling code to remove our custom Annotation/Tag interfaces in favor of just using Doctrine.

I first removed support for the legacy annotations, then added them back in another commit so it can be easily reverted when we remove support. The commit that adds them back duplicates tests for easy removal too.

It's probably easier to review this one commit-by-commit with the middle commit being the mst significant.

  • CHANGELOG.md has been updated

@justinfagnani justinfagnani added this to the 2.0.0 milestone Apr 24, 2017
return {description, tags: parseCustomTags(d.tags)};
}

const namedTags = new Set([
Copy link
Contributor

Choose a reason for hiding this comment

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

namedTags isn't a great name for this.

tagsWithNames and a comment explaining the format expected for these tags?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

}`,
severity: Severity.WARNING,
sourceRange: this.document.sourceRangeForNode(node)!
});
continue;
continue;
Copy link
Contributor

Choose a reason for hiding this comment

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

hm, I think we should fiddle with this a bit until clang-format doesn't mangle it so

Copy link
Contributor Author

Choose a reason for hiding this comment

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

trying, and I'll add a format commit

sourceRange: scannedMixinReference.sourceRange!,
});
continue;
} else if (mixins.size > 1) {
element.warnings.push({
message: `@mixes reference, multiple mixins found ${mixinId}`,
message: `@appliesMixin reference, multiple mixins found ${mixinId}`,
Copy link
Contributor

Choose a reason for hiding this comment

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

wording nit: multiple mixins found with name ${mixinId}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

sourceRange: scannedMixinReference.sourceRange!,
});
continue;
}
const mixin = mixins.values().next().value;
if (!(mixin instanceof PolymerElementMixin)) {
element.warnings.push({
message: `@mixes reference to a non-Mixin ${mixinId}`,
message: `@appliesMixin reference to a non-Mixin ${mixinId}`,
Copy link
Contributor

Choose a reason for hiding this comment

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

non-Polymer mixin?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

@@ -133,6 +132,7 @@ export class Polymer2ElementScanner implements JavaScriptScanner {
const className = element.clazz.namespacedName;
let tagName: string|undefined = undefined;
// TODO(rictic): support `@customElements explicit-tag-name` from jsdoc
// TODO(justinfagnani): do this in this PR?
Copy link
Contributor

Choose a reason for hiding this comment

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

recommend against doing this now, as this whole thing is getting a big rewrite in #625

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ack

jsdoc.getTag(parsedJsdoc, 'pseudoElement', 'name') || undefined;
if (pseudoTag) {
const pseudoTag = jsdoc.getTag(parsedJsdoc, 'pseudoElement');
console.log('pseudoTag', pseudoTag);
Copy link
Contributor

Choose a reason for hiding this comment

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

console.log

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

@justinfagnani
Copy link
Contributor Author

@rictic PTAL

@rictic
Copy link
Contributor

rictic commented Apr 24, 2017

Looks good, but travis is failing on gulp build is failing, maybe on json schema generation? Did any of the analysis format types change with this PR?

Other PRs aren't failing so I don't think this is an upstream thing

@justinfagnani
Copy link
Contributor Author

@rictic Rebased and push -f'ed, but now I need to rebase again :(

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

Successfully merging this pull request may close these issues.

2 participants