Skip to content

Compiler warnings are incorrectly documented, some do not work and some do not exist #4038

Open
@fingerartur

Description

@fingerartur

Looking at the documentation of compiler flags, specifically those, that are OFF by default, there are many problems. Let's break it down:

Some flags have a wrong default value in the wiki

Some flags are not OFF, so their default value in the wiki should either be WARNING or ERROR (not OFF). These flags are:

  • visibility
  • accessControls
  • const
  • constantProperty
  • missingRequire - Maybe this flag should be documented a little more in detail. What module system does it apply to? For example missing import/export statements in ESM modules are type checked by default

Some flags are not recognised by the compiler

  • undefinedNames

Some flags simply do not work

  • unusedPrivateMembers
class Car {
  constructor() {
    /**
     * @private
     */
    this.color = 'red'; // should raise error, but doesn't
  }
}
  • strictPrimitiveOperators
function go() {
  return 555 + 'hello' + {} + new Element() // should raise error, but desn't
}

Sometimes it is very hard to understand what a flag does

  • missingProperties
Warnings about whether a property will ever be defined on an object. Part of type-checking.

What does that mean?

  • strictModuleDepCheck
Warnings about all references potentially violating module dependencies.

What does that mean? What is a violation of module dependencies?

  • typeInvalidation
Warn about properties that cannot be disambiguated when using type based optimizations

What does that mean?

Some flags need more documentation

  • strictMissingProperties
Warnings for missing properties that forbid accessing subclass props off superclasses

In reality though, this flag does so much more. Only the authors know all the details. From what I have been able to reverse engineer, this flag completely changes the behavior of union types, the all type, Element type, etc. I believe this would be worth mentioning in order to facilitate an easier understanding of all that this flag actually does.

Some flags are wrongly documented

  • reportUnknownTypes
Warnings for any place in the code where type is inferred to ?. NOT RECOMMENDED!

This documentation is not entirely true, if I declare a variable of unknown (?) type - so it is declared, as opposed to inferred - the compiler also raises error. So the behavior I see is that it does not apply only to inferred types but also to declared types.

/**
 * @param {?} z // raises error
 */
function hello(z) {
  console.log('hello', z);
}

This is either a mistake in documentation or a bug in the compiler - that depends on what was the intended behavior.

There is also an interesting question: Why does the documentation say it is "NOT RECOMMENDED!"? This would deserve a little more in-depth explanation. It is not immediately obvious why this flag is not recommended. It seems like a useful flag.

Environment

Compiler Version: v20221102

Build command:

java -jar ./scripts/closureCompiler.jar \
  --entry_point=./src/js/index.js \
  --js=./src/**.js \
  --dependency_mode=PRUNE \
  --warning_level=VERBOSE \
  --js_output_file=./dist/bundle.js \
  --module_resolution=WEBPACK \
  --compilation_level=ADVANCED;
  
  # Plus whatever flag I was testing at that moment, e.g.:
  # --jscomp_error=checkDebuggerStatement
  # --jscomp_error=unusedLocalVariables
  # --jscomp_error=reportUnknownTypes
  # --jscomp_error=strictCheckTypes

Metadata

Metadata

Assignees

No one assigned

    Labels

    triage-doneHas been reviewed by someone on triage rotation.

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions