Skip to content
This repository has been archived by the owner on Mar 25, 2021. It is now read-only.

typecheck errors on import break output format (--format json) #2809

Closed
cescoferraro opened this issue May 22, 2017 · 4 comments
Closed

typecheck errors on import break output format (--format json) #2809

cescoferraro opened this issue May 22, 2017 · 4 comments

Comments

@cescoferraro
Copy link

cescoferraro commented May 22, 2017

Bug Report

  • TSLint version: 5.2.0
  • TypeScript version: 2.3.2
  • Running TSLint via: tslint-emacs & cli

TypeScript code being linted

import * as React from "react"
import { Loading, Empty } from "../../shared/components/helpers"
import { isEmpty } from "react-redux-firebase"
import RaisedButton from "material-ui/RaisedButton"

export const Extras = ({ css, app, ROUTER_EMITTER }) => {
    const handleClick = () => {
        ROUTER_EMITTER("/whatever")
    }
    const title = (!isLoaded(app) ?
        <Loading /> : isEmpty(app) ? <Empty /> :
            <h2>{app.title}</h2>)
    return (
        <div className={css}>
            {title}
            <RaisedButton
                onClick={handleClick}
                fullWidth={true}
                label="Go Somewhere"
                primary={true}
            />
        </div>
    )
}

with tslint.json configuration:

{
    "extends": ["tslint:latest", "tslint-react"],
    "rules":{
	"semicolon": [true, "never"],
	"trailing-comma": [
	    true,
	    {
		"multiline": {
		    "objects": "never",
		    "arrays": "never",
		    "functions": "never",
		    "typeLiterals": "ignore"
		}
	    }
	],
	"no-console": [false],
	"no-var-requires": false,
	"no-unused-variable": true,
	"ordered-imports": false 
    }
}

Actual behavior

This was pointed to my by on of flycheck contribuitors here
flycheck/flycheck#1253

I think there is a bug in tslint that output text intead of json even with the json flag when you try to import something that does not exist. Because its working fine with dangling imports see.
It also breaks with other situations like var cesco : string = 33

screen shot 2017-05-22 at 11 45 23

cli

~/go/src/github.com/cescoferraro/achars (develop)
��─ ��  tslint --type-check --format json --project tsconfig.json app/containers/extras.tsx
Error at /Users/cesco/go/src/github.com/cescoferraro/achars/app/containers/extras.tsx:10:21: Cannot find name 'isLoaded'.
Error at app/containers/flycheck_extras.tsx:10:21: Cannot find name 'isLoaded'.

With the error fixed

 ~/go/src/github.com/cescoferraro/achars (develop)
��─ ��  tslint --type-check --format json --project tsconfig.json app/containers/extras.tsx
[]

With a error not on the import lines. as simple as var cesco = 33

~/go/src/github.com/cescoferraro/achars (develop)
��─ ��  tslint --type-check --format json --project tsconfig.json app/containers/extras.tsx
[{"endPosition":{"character":7,"line":9,"position":345},"failure":"Forbidden 'var' keyword, use 'let' or 'const' instead","fix":{"innerStart":342,"innerLength":3,"innerText":"let"},"name":"/Users/cesco/go/src/github.com/cescoferraro/achars/app/containers/extras.tsx","ruleName":"no-var-keyword","ruleSeverity":"ERROR","startPosition":{"character":4,"line":9,"position":342}},{"endPosition":{"character":13,"line":9,"position":351},"failure":"Identifier 'cesco' is never reassigned; use 'const' instead of 'var'.","name":"/Users/cesco/go/src/github.com/cescoferraro/achars/app/containers/extras.tsx","ruleName":"prefer-const","ruleSeverity":"ERROR","startPosition":{"character":8,"line":9,"position":346}},{"endPosition":{"character":13,"line":9,"position":351},"failure":"'cesco' is declared but never used.","name":"/Users/cesco/go/src/github.com/cescoferraro/achars/app/containers/extras.tsx","ruleName":"no-unused-variable","ruleSeverity":"ERROR","startPosition":{"character":8,"line":9,"position":346}}]

emacs

Suspicious state from syntax checker typescript-tslint-cesco: Flycheck checker typescript-tslint-cesco returned non-zero exit code 1, but its output contained no errors: Error at /Users/cesco/go/src/github.com/cescoferraro/achars/app/containers/extras.tsx:10:21: Cannot find name 'isLoaded'.
Error at /Users/cesco/go/src/github.com/cescoferraro/achars/app/containers/flycheck_extras.tsx:10:21: Cannot find name 'isLoaded'.

Try installing a more recent version of typescript-tslint-cesco, and please open a bug report if the issue persists in the latest release.  Thanks!

Expected behavior

always output json

@cescoferraro cescoferraro changed the title typecheck errors on import break type-check errors output formar typecheck errors on import break output format (--format json) May 22, 2017
@ajafff
Copy link
Contributor

ajafff commented May 23, 2017

once #2773 lands, you no longer need --type-check and tslint will not abort on compile errors. That way you always get valid json in the output.

Until then the plugin could just ignore stderr and check if stdout is empty before parsing it as json.

@cescoferraro
Copy link
Author

@ajaff the pr solves my problem. But the bug stand still. When trying this out I accidentally forgot to remove the --type-check flag, and it kept returning plain text. Without it I get json. But if the --project flag enables typecheck, tslint should have the same behavior with or without the flag.

@ajafff
Copy link
Contributor

ajafff commented Jun 2, 2017

@cescoferraro
The failures you get when using --type-check in a project that has type errors, are completely different from lint errors.
We could of course just output them as JSON, but the structure would not match your expectations.
And there's another problem: tslint has not just the JSON formatter, but some more. Most of them have a certain fixed format that wouldn't match the structure of the type errors.

I get your point about the current behavior being wrong, but this is very hard to do right.

On the other hand, when using --type-check now (as of tslint@5.4.0) you explicitly tell tslint to first check for type errors before doing it's own work. One could argue that the different output may be expected in this case...

Maybe the flag will be deprecated at some point in time, that would solve the problem entirely.

@cescoferraro
Copy link
Author

Great explanation! Will close this cause my issue is gone

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

No branches or pull requests

2 participants