Skip to content

Commit

Permalink
Automated g4 rollback of changelist 212058481.
Browse files Browse the repository at this point in the history
*** Reason for rollback ***

The new linter will incorrectly warn on an unqualified template parameter, e.g.

    /** @constructor @template T */ function Foo() {}
    /** @param {T} x */ Foo.prototype.bar = function(x) {};

will warn that the T in @param needs a ! or ?.

Rolling back to prevent a bad linter release.

*** Original change description ***

Extend the CheckRedundantNullabilityModifier linter pass to also check for missing nullability modifiers according to the style guide.

***

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=212075073
  • Loading branch information
tjgq authored and blickly committed Sep 11, 2018
1 parent 5ece03d commit 5df0fa2
Show file tree
Hide file tree
Showing 14 changed files with 290 additions and 395 deletions.
4 changes: 2 additions & 2 deletions src/com/google/javascript/jscomp/DefaultPassConfig.java
Expand Up @@ -51,10 +51,10 @@
import com.google.javascript.jscomp.lint.CheckJSDocStyle; import com.google.javascript.jscomp.lint.CheckJSDocStyle;
import com.google.javascript.jscomp.lint.CheckMissingSemicolon; import com.google.javascript.jscomp.lint.CheckMissingSemicolon;
import com.google.javascript.jscomp.lint.CheckNoMutatedEs6Exports; import com.google.javascript.jscomp.lint.CheckNoMutatedEs6Exports;
import com.google.javascript.jscomp.lint.CheckNullabilityModifiers;
import com.google.javascript.jscomp.lint.CheckNullableReturn; import com.google.javascript.jscomp.lint.CheckNullableReturn;
import com.google.javascript.jscomp.lint.CheckPrimitiveAsObject; import com.google.javascript.jscomp.lint.CheckPrimitiveAsObject;
import com.google.javascript.jscomp.lint.CheckPrototypeProperties; import com.google.javascript.jscomp.lint.CheckPrototypeProperties;
import com.google.javascript.jscomp.lint.CheckRedundantNullabilityModifier;
import com.google.javascript.jscomp.lint.CheckRequiresAndProvidesSorted; import com.google.javascript.jscomp.lint.CheckRequiresAndProvidesSorted;
import com.google.javascript.jscomp.lint.CheckUnusedLabels; import com.google.javascript.jscomp.lint.CheckUnusedLabels;
import com.google.javascript.jscomp.lint.CheckUselessBlocks; import com.google.javascript.jscomp.lint.CheckUselessBlocks;
Expand Down Expand Up @@ -1971,9 +1971,9 @@ protected HotSwapCompilerPass create(AbstractCompiler compiler) {
.add(new CheckInterfaces(compiler)) .add(new CheckInterfaces(compiler))
.add(new CheckJSDocStyle(compiler)) .add(new CheckJSDocStyle(compiler))
.add(new CheckMissingSemicolon(compiler)) .add(new CheckMissingSemicolon(compiler))
.add(new CheckNullabilityModifiers(compiler))
.add(new CheckPrimitiveAsObject(compiler)) .add(new CheckPrimitiveAsObject(compiler))
.add(new CheckPrototypeProperties(compiler)) .add(new CheckPrototypeProperties(compiler))
.add(new CheckRedundantNullabilityModifier(compiler))
.add(new CheckUnusedLabels(compiler)) .add(new CheckUnusedLabels(compiler))
.add(new CheckUselessBlocks(compiler)); .add(new CheckUselessBlocks(compiler));
return combineChecks(compiler, callbacks.build()); return combineChecks(compiler, callbacks.build());
Expand Down
18 changes: 9 additions & 9 deletions src/com/google/javascript/jscomp/DiagnosticGroups.java
Expand Up @@ -31,10 +31,10 @@
import com.google.javascript.jscomp.lint.CheckJSDocStyle; import com.google.javascript.jscomp.lint.CheckJSDocStyle;
import com.google.javascript.jscomp.lint.CheckMissingSemicolon; import com.google.javascript.jscomp.lint.CheckMissingSemicolon;
import com.google.javascript.jscomp.lint.CheckNoMutatedEs6Exports; import com.google.javascript.jscomp.lint.CheckNoMutatedEs6Exports;
import com.google.javascript.jscomp.lint.CheckNullabilityModifiers;
import com.google.javascript.jscomp.lint.CheckNullableReturn; import com.google.javascript.jscomp.lint.CheckNullableReturn;
import com.google.javascript.jscomp.lint.CheckPrimitiveAsObject; import com.google.javascript.jscomp.lint.CheckPrimitiveAsObject;
import com.google.javascript.jscomp.lint.CheckPrototypeProperties; import com.google.javascript.jscomp.lint.CheckPrototypeProperties;
import com.google.javascript.jscomp.lint.CheckRedundantNullabilityModifier;
import com.google.javascript.jscomp.lint.CheckRequiresAndProvidesSorted; import com.google.javascript.jscomp.lint.CheckRequiresAndProvidesSorted;
import com.google.javascript.jscomp.lint.CheckUnusedLabels; import com.google.javascript.jscomp.lint.CheckUnusedLabels;
import com.google.javascript.jscomp.lint.CheckUselessBlocks; import com.google.javascript.jscomp.lint.CheckUselessBlocks;
Expand Down Expand Up @@ -580,11 +580,10 @@ public DiagnosticGroup forName(String name) {
CheckInterfaces.INTERFACE_FUNCTION_NOT_EMPTY, CheckInterfaces.INTERFACE_FUNCTION_NOT_EMPTY,
CheckInterfaces.INTERFACE_SHOULD_NOT_TAKE_ARGS, CheckInterfaces.INTERFACE_SHOULD_NOT_TAKE_ARGS,
CheckMissingSemicolon.MISSING_SEMICOLON, CheckMissingSemicolon.MISSING_SEMICOLON,
CheckNullabilityModifiers.MISSING_NULLABILITY_MODIFIER_JSDOC,
CheckNullabilityModifiers.REDUNDANT_NULLABILITY_MODIFIER_JSDOC,
CheckPrimitiveAsObject.NEW_PRIMITIVE_OBJECT, CheckPrimitiveAsObject.NEW_PRIMITIVE_OBJECT,
CheckPrimitiveAsObject.PRIMITIVE_OBJECT_DECLARATION, CheckPrimitiveAsObject.PRIMITIVE_OBJECT_DECLARATION,
CheckPrototypeProperties.ILLEGAL_PROTOTYPE_MEMBER, CheckPrototypeProperties.ILLEGAL_PROTOTYPE_MEMBER,
CheckRedundantNullabilityModifier.REDUNDANT_NULLABILITY_MODIFIER_JSDOC,
CheckRequiresAndProvidesSorted.DUPLICATE_REQUIRE, CheckRequiresAndProvidesSorted.DUPLICATE_REQUIRE,
CheckRequiresAndProvidesSorted.REQUIRES_NOT_SORTED, CheckRequiresAndProvidesSorted.REQUIRES_NOT_SORTED,
CheckRequiresAndProvidesSorted.PROVIDES_NOT_SORTED, CheckRequiresAndProvidesSorted.PROVIDES_NOT_SORTED,
Expand Down Expand Up @@ -660,8 +659,7 @@ public DiagnosticGroup forName(String name) {
// the file level is by using a whitelist file. // the file level is by using a whitelist file.
@GwtIncompatible("Conformance") @GwtIncompatible("Conformance")
public static final DiagnosticGroup CONFORMANCE_VIOLATIONS = public static final DiagnosticGroup CONFORMANCE_VIOLATIONS =
DiagnosticGroups.registerGroup( DiagnosticGroups.registerGroup("conformanceViolations",
"conformanceViolations",
CheckConformance.CONFORMANCE_VIOLATION, CheckConformance.CONFORMANCE_VIOLATION,
CheckConformance.CONFORMANCE_POSSIBLE_VIOLATION); CheckConformance.CONFORMANCE_POSSIBLE_VIOLATION);


Expand All @@ -672,14 +670,16 @@ public DiagnosticGroup forName(String name) {


public static final DiagnosticGroup MISSING_POLYFILL = public static final DiagnosticGroup MISSING_POLYFILL =
DiagnosticGroups.registerGroup( DiagnosticGroups.registerGroup(
"missingPolyfill", RewritePolyfills.INSUFFICIENT_OUTPUT_VERSION_ERROR); "missingPolyfill",
RewritePolyfills.INSUFFICIENT_OUTPUT_VERSION_ERROR);


// For internal use only, so there are no constants for these groups. // For internal use only, so there are no constants for these groups.
static { static {
DiagnosticGroups.registerGroup( DiagnosticGroups.registerGroup("invalidProvide",
"invalidProvide", ProcessClosurePrimitives.INVALID_PROVIDE_ERROR); ProcessClosurePrimitives.INVALID_PROVIDE_ERROR);


DiagnosticGroups.registerGroup("es6Typed", RhinoErrorReporter.MISPLACED_TYPE_SYNTAX); DiagnosticGroups.registerGroup("es6Typed",
RhinoErrorReporter.MISPLACED_TYPE_SYNTAX);


DiagnosticGroups.registerDeprecatedGroup("duplicateZipContents"); DiagnosticGroups.registerDeprecatedGroup("duplicateZipContents");
} }
Expand Down
4 changes: 2 additions & 2 deletions src/com/google/javascript/jscomp/LintPassConfig.java
Expand Up @@ -22,9 +22,9 @@
import com.google.javascript.jscomp.lint.CheckInterfaces; import com.google.javascript.jscomp.lint.CheckInterfaces;
import com.google.javascript.jscomp.lint.CheckJSDocStyle; import com.google.javascript.jscomp.lint.CheckJSDocStyle;
import com.google.javascript.jscomp.lint.CheckMissingSemicolon; import com.google.javascript.jscomp.lint.CheckMissingSemicolon;
import com.google.javascript.jscomp.lint.CheckNullabilityModifiers;
import com.google.javascript.jscomp.lint.CheckPrimitiveAsObject; import com.google.javascript.jscomp.lint.CheckPrimitiveAsObject;
import com.google.javascript.jscomp.lint.CheckPrototypeProperties; import com.google.javascript.jscomp.lint.CheckPrototypeProperties;
import com.google.javascript.jscomp.lint.CheckRedundantNullabilityModifier;
import com.google.javascript.jscomp.lint.CheckRequiresAndProvidesSorted; import com.google.javascript.jscomp.lint.CheckRequiresAndProvidesSorted;
import com.google.javascript.jscomp.lint.CheckUnusedLabels; import com.google.javascript.jscomp.lint.CheckUnusedLabels;
import com.google.javascript.jscomp.lint.CheckUselessBlocks; import com.google.javascript.jscomp.lint.CheckUselessBlocks;
Expand Down Expand Up @@ -73,7 +73,7 @@ protected CompilerPass create(AbstractCompiler compiler) {
new CheckSuper(compiler), new CheckSuper(compiler),
new CheckPrimitiveAsObject(compiler), new CheckPrimitiveAsObject(compiler),
new ClosureCheckModule(compiler), new ClosureCheckModule(compiler),
new CheckNullabilityModifiers(compiler), new CheckRedundantNullabilityModifier(compiler),
new CheckRequiresAndProvidesSorted(compiler), new CheckRequiresAndProvidesSorted(compiler),
new CheckSideEffects( new CheckSideEffects(
compiler, /* report */ true, /* protectSideEffectFreeCode */ false), compiler, /* report */ true, /* protectSideEffectFreeCode */ false),
Expand Down
164 changes: 0 additions & 164 deletions src/com/google/javascript/jscomp/lint/CheckNullabilityModifiers.java

This file was deleted.

@@ -0,0 +1,118 @@
/*
* Copyright 2018 The Closure Compiler Authors.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package com.google.javascript.jscomp.lint;

import com.google.common.collect.ImmutableSet;
import com.google.javascript.jscomp.AbstractCompiler;
import com.google.javascript.jscomp.CompilerPass;
import com.google.javascript.jscomp.DiagnosticType;
import com.google.javascript.jscomp.NodeTraversal;
import com.google.javascript.jscomp.NodeTraversal.AbstractPostOrderCallback;
import com.google.javascript.jscomp.NodeUtil;
import com.google.javascript.rhino.JSDocInfo;
import com.google.javascript.rhino.JSTypeExpression;
import com.google.javascript.rhino.Node;
import com.google.javascript.rhino.Token;

/**
* Checks for redundant nullability modifiers.
*
* <p>A nullability modifier '!' is redundant if it precedes a value type or a record literal.
*/
public class CheckRedundantNullabilityModifier extends AbstractPostOrderCallback
implements CompilerPass {

/** The diagnostic for a redundant nullability modifier. */
public static final DiagnosticType REDUNDANT_NULLABILITY_MODIFIER_JSDOC =
DiagnosticType.warning(
"JSC_REDUNDANT_NULLABILITY_MODIFIER_JSDOC",
"Type is already not null. Please remove the redundant '!'.");

private static final ImmutableSet<String> PRIMITIVE_TYPE_NAMES =
ImmutableSet.of("boolean", "number", "string", "symbol", "undefined");

private final AbstractCompiler compiler;

public CheckRedundantNullabilityModifier(AbstractCompiler compiler) {
this.compiler = compiler;
}

@Override
public void process(Node externs, Node root) {
NodeTraversal.traverseRoots(compiler, this, externs, root);
}

@Override
public void visit(final NodeTraversal t, final Node n, final Node p) {
final JSDocInfo info = n.getJSDocInfo();
if (info == null) {
return;
}
if (info.hasType()) {
checkTypeExpression(t, info.getType(), false);
}
for (String param : info.getParameterNames()) {
if (info.hasParameterType(param)) {
checkTypeExpression(t, info.getParameterType(param), false);
}
}
if (info.hasReturnType()) {
checkTypeExpression(t, info.getReturnType(), false);
}
if (info.hasEnumParameterType()) {
checkTypeExpression(t, info.getEnumParameterType(), false);
}
if (info.hasTypedefType()) {
checkTypeExpression(t, info.getTypedefType(), false);
}
// JSDocInfoParser automatically prepends a '!' to the type associated with a @this annotation.
// It should be ignored since it doesn't actually exist in the source file.
if (info.hasThisType()) {
checkTypeExpression(t, info.getThisType(), true);
}
// We don't need to check @extends and @implements annotations since they never refer to a
// primitive type.
// TODO(tjgq): Should we check @throws annotations? The Google style guide forbids throwing a
// primitive type, so the warning would be somewhat misguided.
}

private static void checkTypeExpression(
final NodeTraversal t, JSTypeExpression expr, boolean isTopLevelBangAllowed) {
NodeUtil.visitPreOrder(
expr.getRoot(),
node -> {
Node parent = node.getParent();
Node grandparent = parent != null ? parent.getParent() : null;

boolean hasModifier = parent != null && parent.getToken() == Token.BANG;
boolean isModifierRedundant =
isPrimitiveType(node) || node.isFunction() || isRecordLiteral(node);
boolean isRedundantModifierAllowed = isTopLevelBangAllowed && grandparent == null;

if (hasModifier && isModifierRedundant && !isRedundantModifierAllowed) {
t.report(parent, REDUNDANT_NULLABILITY_MODIFIER_JSDOC);
}
});
}

private static boolean isPrimitiveType(Node node) {
return node.isString() && PRIMITIVE_TYPE_NAMES.contains(node.getString());
}

private static boolean isRecordLiteral(Node node) {
return node.getToken() == Token.LC;
}
}

0 comments on commit 5df0fa2

Please sign in to comment.