Skip to content

Commit 71ede47

Browse files
lauraharkercopybara-github
authored andcommitted
Split up a long method in CheckMissingRequires
This is a no-op but simplifies the control flow, especially because in a followup change I'm adding more code to "checkMissingRequireThroughShortName". Also delete unused "getRootNode" method. PiperOrigin-RevId: 721922168
1 parent 6fc2d0f commit 71ede47

File tree

1 file changed

+93
-61
lines changed

1 file changed

+93
-61
lines changed

src/com/google/javascript/jscomp/CheckMissingRequires.java

+93-61
Original file line numberDiff line numberDiff line change
@@ -301,25 +301,49 @@ private void visitQualifiedName(
301301

302302
Var var = t.getScope().getVar(rootName);
303303
if (var != null && var.getScope().isLocal()) {
304-
// Currently, tsickle can introduce these
305-
// TODO(b/333952917): Remove this once tsickle is fixed.
306-
if (isTypeScriptSource(n)) {
307-
return;
308-
}
304+
checkMissingRequireThroughShortName(t, n, var, currentFile, qualifiedName, isStrongReference);
305+
} else {
306+
checkMissingRequireThroughFullyQualifiedName(
307+
t, n, currentFile, qualifiedName, isStrongReference);
308+
}
309+
}
309310

310-
if (!currentFile.isModule()) {
311-
// Don't worry about aliases outside of module files for now.
312-
return;
313-
}
311+
/**
312+
* Checks if a reference to a local variable should actually trigger a missing require warning.
313+
*
314+
* <p>This only warns within goog.scope and module bodies, in cases where there's a local variable
315+
* that's actually an alias of some import.
316+
*
317+
* @param n the node representing the fully qualified name being checked, starting with localVar
318+
* @param localVar the root of the qualified name
319+
* @param qualifiedName some fully qualified name to check, starting with localVar
320+
* @param isStrongReference whether this is a non-type-only reference
321+
*/
322+
private void checkMissingRequireThroughShortName(
323+
NodeTraversal t,
324+
Node n,
325+
Var localVar,
326+
ModuleMetadata currentFile,
327+
QualifiedName qualifiedName,
328+
boolean isStrongReference) {
329+
// TODO(b/333952917): Remove this once tsickle is fixed.
330+
if (isTypeScriptSource(n)) {
331+
return;
332+
}
314333

315-
// The qualified name *is* the root name if it is "simple"
316-
if (qualifiedName.isSimple()) {
317-
// It is explicitly imported and we have already validated the import in
318-
// `visitMaybeDeclaration`
319-
return;
320-
}
334+
if (!currentFile.isModule()) {
335+
// Don't worry about aliases outside of module files for now.
336+
return;
337+
}
321338

322-
NodeUtil.GoogRequire require = NodeUtil.getGoogRequireInfo(var);
339+
// The qualified name *is* the root name if it is "simple"
340+
if (qualifiedName.isSimple()) {
341+
// It is explicitly imported and we have already validated the import in
342+
// `visitMaybeDeclaration`
343+
return;
344+
}
345+
346+
NodeUtil.GoogRequire require = NodeUtil.getGoogRequireInfo(localVar);
323347
// TODO: validate that this is the correct thing to do
324348
if (require == null || require.namespace().equals("wiz")) {
325349
// It is a local name, not an import.
@@ -339,54 +363,69 @@ private void visitQualifiedName(
339363
// Verify namespace usage
340364
QualifiedName normalizeQualifiedName = normalizeQualifiedName(qualifiedName, require);
341365

342-
// TESTCASES, where A.B should not be used through A:
343-
// checked here
344-
// X const A = require('A'); ref(A.B); // error, missing require A.B
345-
// X const A = require('A'); const B = require('B'); ref(A.B); // error, incorrect ref
346-
// checked in declaration
347-
// X const {B} = require('A'); // error bad import
348-
//
349-
350-
// Look for the longest prefix match against a provided namespace.
351-
for (QualifiedName subName = normalizeQualifiedName;
352-
subName != null;
353-
subName = subName.getOwner()) {
354-
String namespace = subName.join();
355-
if (isAllowedNamespace(currentFile, namespace)) {
356-
return;
357-
}
366+
// TESTCASES, where A.B should not be used through A:
367+
// checked here
368+
// X const A = require('A'); ref(A.B); // error, missing require A.B
369+
// X const A = require('A'); const B = require('B'); ref(A.B); // error, incorrect ref
370+
// checked in declaration
371+
// X const {B} = require('A'); // error bad import
372+
//
358373

359-
ModuleMetadata requiredFile = moduleByNamespace.get(namespace);
360-
if (requiredFile == null) {
361-
// Not a known namespace check the parent
362-
continue;
363-
}
374+
// Look for the longest prefix match against a provided namespace.
375+
for (QualifiedName subName = normalizeQualifiedName;
376+
subName != null;
377+
subName = subName.getOwner()) {
378+
String namespace = subName.join();
379+
if (isAllowedNamespace(currentFile, namespace)) {
380+
return;
381+
}
364382

365-
// TODO: report on the node that needs rewritten, include the namespace that needs
366-
// to be added:
367-
// Autofix: add the require if necessary, rewrite to be the full namespace, and let fixjs
368-
// fix
369-
// it up. Write a different message if the namespace is already imported vs missing.
383+
ModuleMetadata requiredFile = moduleByNamespace.get(namespace);
384+
if (requiredFile == null) {
385+
// Not a known namespace check the parent
386+
continue;
387+
}
370388

371-
if (!hasAcceptableRequire(currentFile, subName, requiredFile, isStrongReference)
372-
|| originalRequiredFile != requiredFile) {
389+
// TODO: report on the node that needs rewritten, include the namespace that needs
390+
// to be added:
391+
// Autofix: add the require if necessary, rewrite to be the full namespace, and let fixjs
392+
// fix
393+
// it up. Write a different message if the namespace is already imported vs missing.
373394

374-
// `originalRequiredFile != requiredFile` then the file is being referenced through
375-
// the wrong namespace even though it is being `goog.require`d in the file.
395+
if (!hasAcceptableRequire(currentFile, subName, requiredFile, isStrongReference)
396+
|| originalRequiredFile != requiredFile) {
376397

377-
DiagnosticType toReport =
378-
isStrongReference
379-
? INDIRECT_NAMESPACE_REF_REQUIRE
380-
: INDIRECT_NAMESPACE_REF_REQUIRE_TYPE;
381-
t.report(n, toReport, namespace);
382-
}
398+
// `originalRequiredFile != requiredFile` then the file is being referenced through
399+
// the wrong namespace even though it is being `goog.require`d in the file.
383400

384-
// We found the imported namespace: done
385-
return;
401+
DiagnosticType toReport =
402+
isStrongReference
403+
? INDIRECT_NAMESPACE_REF_REQUIRE
404+
: INDIRECT_NAMESPACE_REF_REQUIRE_TYPE;
405+
t.report(n, toReport, namespace);
386406
}
387407

408+
// We found the imported namespace: done
388409
return;
389410
}
411+
}
412+
413+
/**
414+
* Checks if a reference to a global qualified name should trigger a missing require warning
415+
*
416+
* <p>Here, "global qualified name" means that the root object for the name is not defined in some
417+
* local module or function scope, but is global.
418+
*
419+
* @param n the node representing the fully qualified name being checked
420+
* @param qualifiedName some global fully qualified name to check
421+
* @param isStrongReference whether this is a non-type-only reference
422+
*/
423+
private void checkMissingRequireThroughFullyQualifiedName(
424+
NodeTraversal t,
425+
Node n,
426+
ModuleMetadata currentFile,
427+
QualifiedName qualifiedName,
428+
boolean isStrongReference) {
390429

391430
// Look for the longest prefix match against a provided namespace.
392431
for (QualifiedName subName = qualifiedName; subName != null; subName = subName.getOwner()) {
@@ -426,13 +465,6 @@ private void visitQualifiedName(
426465
}
427466
}
428467

429-
static Node getRootNode(Node n) {
430-
while (n.isGetProp()) {
431-
n = n.getFirstChild();
432-
}
433-
return n;
434-
}
435-
436468
/**
437469
* Given a qualified name with local root name and a import that defines that name, transform that
438470
* name so that it is a fully qualified name.

0 commit comments

Comments
 (0)