Salsa does not warn or help when passing in wrong argument type

[egamma]

From @alexandrudima on January 27, 2016 13:1

Testing #2218

/**
 * @param {string} name
 */
function howdy(name) {

}

howdy(3);

even if it knows a string is expected:

Copied from original issue: microsoft/vscode#2442

[egamma]

From @billti on January 27, 2016 16:29

Correct. We explicitly decided to give no warnings/errors other than syntax errors for JavaScript code. If warnings on incorrect type usage are required, we'd need to spec how and when these would occur in JavaScript, else being based on TypeScript, code such as the below would also be a type error:

var x = 10;
x = "ten";

var y = {a:true};
y.b = false;

[egamma]

In the existing Code JS language service we gave warnings for the first example above and we didn't receive complaints.

We have also warned about undefined variables, which had issues when users where also using a linter in their project and they defined a global as a global in their linter configuration. The confusion was that they define the global in linter configuration, but our JS service still warned about the global.

[billti]

@egamma Are you saying you would like TypeScript type errors (as JavaScript warnings) in all scenarios? Or is there a subset you care about we should filter and allow through?

[egamma]

@billti in the existing JS service we have exposed more type errors as warnings and allowed the user to disable them using settings. Below are the lint related settings we had.

Since TS is starting to have some linting related checks (e.g. unreachable code) it suggest to also expose some of these warnings. What I would do differently now, is that they should all be opt-in/off by default.

+@jrieken

    // Don't spare curly brackets.
    "javascript.validate.lint.curlyBracketsMustNotBeOmitted": "ignore",

    // Empty block should have a comment.
    "javascript.validate.lint.emptyBlocksWithoutComment": "ignore",

    // Use '!==' and '===' instead of '!=' and '=='.
    "javascript.validate.lint.comparisonOperatorsNotStrict": "ignore",

    // Missing semicolon.
    "javascript.validate.lint.missingSemicolon": "ignore",

    // Unexpected output of the 'typeof' operator.
    "javascript.validate.lint.unknownTypeOfResults": "warning",

    // Semicolon instead of block.
    "javascript.validate.lint.semicolonsInsteadOfBlocks": "ignore",

    // Function inside loop.
    "javascript.validate.lint.functionsInsideLoops": "ignore",

    // Function with lowercase name used as constructor.
    "javascript.validate.lint.newOnLowercaseFunctions": "warning",

    // Looks for mistyped triple-slash references.
    "javascript.validate.lint.tripleSlashReferenceAlike": "warning",

    // Unused local variable.
    "javascript.validate.lint.unusedVariables": "warning",

    // Unused local function.
    "javascript.validate.lint.unusedFunctions": "ignore",

    // Parameters don't match a function signature
    "javascript.validate.lint.parametersDontMatchSignature": "ignore",

    // Don't re-declare a variable and change its type.
    "javascript.validate.lint.redeclaredVariables": "warning",

    // Don't use an undeclared variable.
    "javascript.validate.lint.undeclaredVariables": "warning",

    // Don't use an unknown property.
    "javascript.validate.lint.unknownProperty": "ignore",

    // Don't require an unknown module.
    "javascript.validate.lint.unknownModule": "ignore",

    // Don't re-declare a variable type by an assignment.
    "javascript.validate.lint.forcedTypeConversion": "warning",

    // Only use numbers for arithmetic operations.
    "javascript.validate.lint.mixedTypesArithmetics": "warning",

    // Don't use instanceof with primitive types.
    "javascript.validate.lint.primitivesInInstanceOf": "error",

    // Function with return statement used as constructor.
    "javascript.validate.lint.newOnReturningFunctions": "warning",

``

[jrieken]

Today, we ignore most semantic errors unless they are from a special hand-tuned set and configured to be either a warning or error: https://github.com/Microsoft/vscode/blob/master/src/vs/languages/typescript/common/features/diagnostics.ts#L92

[alexdima]

@billti I would classify possible squiggles in two buckets:
A. those I can fix with JS + JSDoc syntax
B. those I cannot fix ever

var x = 10;
x = "ten";

var y = {a:true};
y.b = false;

The ones above fall into the second bucket, I cannot help Salsa in any way to make the squiggly go away without rewriting my code to do the assignment in a different statement than the declaration.

I would also classify the types further:

1. those inferred by TypeScript from program source code => they propagate as "inferred"
2. those explicitly introduced by the programmer via JSDoc => they propagate as "forced"

• a "forced" + "inferred" => "inferred"

I would then show errors that fall into the buckets A and 2.

[billti]

@alexandrudima Re "those you cannot ever fix": You can fix both of the above with JsDoc comments, as the first is a union type and the second contains an optional property, e.g. the below is valid from a type system perspective:

/** @type {(number | string)} */
var x = 10;
x = "ten";


/** @type {{a: boolean, b: boolean=}} */
var y = {a:true};
y.b = false;

We discussed the broader question this morning of our design here, and in general believe we should:

a) expose a set of warning of the type outlined above from Salsa
b) provide some type of distinction between errors and warnings
c) preferably provide a way to opt in/out to them per project

I'll open up an issue to address the overall topic and link to this bug.

[mhegazy]

closing in favor of #6802