Do not emit @type if @define is specified

src/jsdoc.ts

@@ -150,15 +150,24 @@ const JSDOC_TAGS_INPUT_BLACKLIST = new Set([
  'record', 'static', 'template', 'this', 'type', 'typedef',
 ]);
 
+/** What to do with types found in JSDoc @tags. */
+enum TagWithTypePolicy {
+ FORBIDDEN,
+ OPTIONAL,
+ REQUIRED,
+}
+
 /**
  * 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.
  */
-const JSDOC_TAGS_WITH_TYPES = new Set([
- 'const',
- 'export',
- 'param',
- 'return',
+const JSDOC_TAGS_WITH_TYPES = new Map([
+ ['const', TagWithTypePolicy.FORBIDDEN],
+ ['define', TagWithTypePolicy.OPTIONAL], // NOTE: this could be made forbidden later.
+ ['export', TagWithTypePolicy.FORBIDDEN],
+ ['param', TagWithTypePolicy.FORBIDDEN],
+ ['return', TagWithTypePolicy.FORBIDDEN],
+ ['suppress', TagWithTypePolicy.REQUIRED],
 ]);
 
 /**
@@ -234,17 +243,21 @@ 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 (tagName === 'suppress') {
- const suppressMatch = text.match(/^\{(.*)\}(.*)$/);
- if (!suppressMatch) {
- warnings.push(`malformed @suppress tag: "${text}"`);
+ } else if (JSDOC_TAGS_WITH_TYPES.has(tagName)) {
+ const policy = JSDOC_TAGS_WITH_TYPES.get(tagName);
+ if (policy === TagWithTypePolicy.FORBIDDEN && text[0] === '{') {
+ warnings.push(
+ `the type annotation on @${tagName} is redundant with its TypeScript type, ` +
+ `remove the {...} part`);
+ continue;
+ }
+ const typeMatch = text.match(/^\{(.*)\}(.*)$/);
+ if (!typeMatch) {
+ if (policy === TagWithTypePolicy.REQUIRED) {
+ warnings.push(`malformed @${tagName} tag: "${text}"`);
+ }
  } else {
- [, type, text] = suppressMatch;
+ [, type, text] = typeMatch;
  }
  } 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,19 @@ 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 do not add @type, but instead verify that any type is
+ // correct. Specifying any type at all here may become an error later.
+ const defineTag = localTags.find(({tagName}) => tagName === 'define');
+ if (!defineTag) {
+ localTags.push({tagName: 'type', type: typeStr});
+ } else if (defineTag.type !== typeStr) {
+ if (defineTag.type !== undefined) {
+ moduleTypeTranslator.error(
+ varStmt,
+ `incorrect type in @define: found '${defineTag.type}' required '${typeStr}'`);
+ }
+ defineTag.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(140,1): error TS0: incorrect type in @define: found 'boolean' required 'number'
 /**
  * @fileoverview added by tsickle
  * @suppress {checkTypes,extraRequire,missingOverride,missingReturn,unusedPrivateMembers,uselessCode} checked by tsc
@@ -166,3 +167,15 @@ Polymer({ behaviors: [(/** @type {?} */ ('test'))] });
  */
 class Foo {
 }
+/**
+ * @define {string}
+ */
+const GOOD_DEFINE = 'x';
+/**
+ * @define {number}
+ */
+const BAD_DEFINE = 42;
+/**
+ * @define {boolean}
+ */
+const MISSING_DEFINE = false;

test_files/jsdoc/jsdoc.ts

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