Do not emit @type if @define is specified (#1003)

Instead, disallow types in `@define` and fill in the TypeScript type.

src/jsdoc.ts

@@ -151,11 +151,13 @@ const JSDOC_TAGS_INPUT_BLACKLIST = new Set([
 ]);
 
 /**
- * A list of JSDoc @tags that might include a {type} after them. Only banned when a type is passed.
- * Note that this does not include tags that carry a non-type system type, e.g. \@suppress.
+ * JSDoc \@tags that might include a {type} after them. Specifying a type is forbidden, since it
+ * would collide with TypeScript's type information. If a type *is* given, the entire tag will be
+ * ignored.
  */
 const JSDOC_TAGS_WITH_TYPES = new Set([
  'const',
+ 'define',
  'export',
  'param',
  'return',
@@ -234,17 +236,19 @@ export function parseContents(commentText: string): ParsedJSDocComment|null {
  // Drop it without any warning. (We also don't ensure its correctness.)
  continue;
  }
- } else if (JSDOC_TAGS_WITH_TYPES.has(tagName) && text[0] === '{') {
- warnings.push(
- `the type annotation on @${tagName} is redundant with its TypeScript type, ` +
- `remove the {...} part`);
- continue;
+ } else if (JSDOC_TAGS_WITH_TYPES.has(tagName)) {
+ if (text[0] === '{') {
+ warnings.push(
+ `the type annotation on @${tagName} is redundant with its TypeScript type, ` +
+ `remove the {...} part`);
+ continue;
+ }
  } else if (tagName === 'suppress') {
- const suppressMatch = text.match(/^\{(.*)\}(.*)$/);
- if (!suppressMatch) {
- warnings.push(`malformed @suppress tag: "${text}"`);
+ const typeMatch = text.match(/^\{(.*)\}(.*)$/);
+ if (typeMatch) {
+ [, type, text] = typeMatch;
  } else {
- [, type, text] = suppressMatch;
+ warnings.push(`malformed @${tagName} tag: "${text}"`);
  }
  } else if (tagName === 'dict') {
  warnings.push('use index signatures (`[k: string]: type`) instead of @dict');

src/jsdoc_transformer.ts

@@ -602,7 +602,8 @@ export function jsdocTransformer(
  tags = null;
  }
  // Add an @type for plain identifiers, but not for bindings patterns (i.e. object or array
- // destructuring) - those do not have a syntax in Closure.
+ // destructuring - those do not have a syntax in Closure) or @defines, which already
+ // declare their type.
  if (ts.isIdentifier(decl.name)) {
  // For variables that are initialized and use a blacklisted type, do not emit a type at
  // all. Closure Compiler might be able to infer a better type from the initializer than
@@ -615,7 +616,13 @@ export function jsdocTransformer(
  // getOriginalNode(decl) is required because the type checker cannot type check
  // synthesized nodes.
  const typeStr = moduleTypeTranslator.typeToClosure(ts.getOriginalNode(decl));
- localTags.push({tagName: 'type', type: typeStr});
+ // If @define is present then add the type to it, rather than adding a normal @type.
+ const defineTag = localTags.find(({tagName}) => tagName === 'define');
+ if (defineTag) {
+ defineTag.type = typeStr;
+ } else {
+ localTags.push({tagName: 'type', type: typeStr});
+ }
  }
  }
  const newStmt = ts.createVariableStatement(

test_files/jsdoc/jsdoc.js

@@ -16,6 +16,7 @@
 // test_files/jsdoc/jsdoc.ts(77,1): warning TS0: @extends annotations are redundant with TypeScript equivalents
 // @implements annotations are redundant with TypeScript equivalents
 // test_files/jsdoc/jsdoc.ts(84,3): warning TS0: @constructor annotations are redundant with TypeScript equivalents
+// test_files/jsdoc/jsdoc.ts(132,1): warning TS0: the type annotation on @define is redundant with its TypeScript type, remove the {...} part
 /**
  * @fileoverview added by tsickle
  * @suppress {checkTypes,extraRequire,missingOverride,missingReturn,unusedPrivateMembers,uselessCode} checked by tsc
@@ -166,3 +167,13 @@ Polymer({ behaviors: [(/** @type {?} */ ('test'))] });
  */
 class Foo {
 }
+/** @type {number} */
+const DEFINE_WITH_JSDOC_TYPE = 42;
+/**
+ * @define {boolean}
+ */
+const DEFINE_WITH_INFERRED_TYPE = false;
+/**
+ * @define {string}
+ */
+const DEFINE_WITH_DECLARED_TYPE = 'y';

test_files/jsdoc/jsdoc.ts

@@ -128,3 +128,18 @@ Polymer({behaviors: ['test' as any]});
  * @template T2 Another user comment.
  */
 class Foo<T1, T2> {}
+
+/**
+ * @define {boolean}
+ */
+const DEFINE_WITH_JSDOC_TYPE = 42;
+
+/**
+ * @define
+ */
+const DEFINE_WITH_INFERRED_TYPE = false;
+
+/**
+ * @define
+ */
+const DEFINE_WITH_DECLARED_TYPE: string = 'y';