Skip to content
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

Suspicious @suppress #3517

Closed
rachel-fenichel opened this issue Dec 14, 2019 · 1 comment
Closed

Suspicious @suppress #3517

rachel-fenichel opened this issue Dec 14, 2019 · 1 comment

Comments

@rachel-fenichel
Copy link
Collaborator

What's the scope of this suppress? https://github.com/google/blockly/blob/master/core/xml.js#L379

  if (xml instanceof Blockly.Workspace) {
    var swap = xml;
    // Closure Compiler complains here because the arguments are reversed.
    /** @suppress {checkTypes} */
    xml = workspace;
    workspace = swap;
    console.warn('Deprecated call to Blockly.Xml.domToWorkspace, ' +
                 'swap the arguments.');
  }

The documentation on @suppress says it rarely operates on a single-line basis: https://github.com/google/closure-compiler/wiki/@suppress-annotations#other-locations

Does that mean that this applies for the rest of the lines of the function? If so, should it be in the function's jsdoc instead?

@rachel-fenichel rachel-fenichel added issue: bug Describes why the code or behaviour is wrong issue: triage Issues awaiting triage by a Blockly team member type: cleanup type: question and removed issue: triage Issues awaiting triage by a Blockly team member issue: bug Describes why the code or behaviour is wrong labels Dec 14, 2019
@samelhusseini
Copy link
Contributor

Based on the documentation checkTypes is not a suppress annotation that fits outside of the method's JSDOC block, which implicitly means it applies to the entire method only. Also based on the documentation, the above suppression doesn't work at all.

However, I did some testing and that checkTypes suppression annotation is in fact only applying to the following statement only and not the entire method which is what we want there.

I can follow up with the closure compiler team about the documentation, but as far as I can tell this is WAI.

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

No branches or pull requests

2 participants