Skip to content
This repository has been archived by the owner on Jul 8, 2019. It is now read-only.

Pass --project and --type-check parameters to tslint #75

Closed
NikitaEgorov opened this issue Dec 13, 2016 · 17 comments
Closed

Pass --project and --type-check parameters to tslint #75

NikitaEgorov opened this issue Dec 13, 2016 · 17 comments

Comments

@NikitaEgorov
Copy link
Contributor

How can i pass --project and --type-check to tslint?

@Pablissimo
Copy link
Owner

Can't just now - I can probably add a configuration parameter to allow extra arguments to be sent. Or specifically add configuration options for those two params...

I should have a little time over Christmas while hiding from the in-laws to have a look.

@Pablissimo
Copy link
Owner

So this is an interesting one - there's an outstanding defect on tslint that essentially means that if you supply both the --project parameter and a list of files, the list of files is ignored and tslint only pays attention to the files listed in the project json file.

This is tricky for us for two reasons:

  • We currently batch files to send to tslint so they don't end up creating a command line that's too long on systems with small limits - but if tslint's ignoring the set of files it's sent and always using the project file then we're doing loads of work in tslint then ignoring the results
  • People using the --project option would need to maintain both their sonar-project.properties file and their tsconfig.json file to make sure they're pointing to the same sets of files unless the above is fixed

Any feeling on what a sensible approach would be? Best thinking I have so far is, if you've specified a path to tsconfig.json:

  • Turn off batching, no longer send a list of files to tslint but trust that tsconfig.json has been setup right
  • If tslint reports lint violations for files that aren't in our analysis set as defined by sonar-project.properties, ignore those violations

@Pablissimo
Copy link
Owner

Also - what would you want to achieve by passing --type-check to tslint that your existing CI build isn't capturing? There's no output from tslint that I can supply to SonarQube if a type-check fails, so far as I know, so it'll just abort the SQ analysis entirely if there are any typing errors. Might be fine, just wondering what the use-case is?

@sevaru
Copy link

sevaru commented Jan 10, 2017

Hi there, thank you for your work @Pablissimo.

"--type-check" with "--project" is required for alot of rules that exists in tslint now.
For example: "no void expression", "promise function async", "no for in array", etc.

As I know tslint supports glob pattern for files from cli (e.g tslint './**/*.ts') as mentioned there palantir/tslint#827, so it is not required to specify files directly.

@Pablissimo
Copy link
Owner

Got it. Have started along the lines of my original proposal since it seems to fit well enough - travelling the next week but should have time in airports and the like to bash through I reckon.

@Pablissimo
Copy link
Owner

Give this a bash - will become v0.99 (or v1.0 /o) and get merged to trunk if it works well enough for you - seems to for my local test projects.

sonar-typescript-plugin-0.99-SNAPSHOT.zip

New parameters are required:

  • sonar.ts.tslintprojectpath - set to probably 'tsconfig.json' or whatever your project file is
  • sonar.ts.tslinttypecheck - true/false, defaults to false to send the --type-check parameter too

A note on its behaviour at the minute - if the project fails to type-check you'll not get any issues reported just now, because tslint just fails hard. I need to tidy up that handling before things go live for real, I think.

@nixel2007
Copy link

sonar.ts.tslintprojectpath

do you set a default value for this parameter?
Like ./tsconfig.json?

@nixel2007
Copy link

And what about to use camelCase for parameters' names? :)

@Pablissimo
Copy link
Owner

Didn't set a default for it, because I can't then tell whether or not to use it if it happens to exist on disk (for example, I've got projects with a tsconfig.json that I'd sooner not use for linting at the mo). Essentially it prevents changing the plugin's behaviour without the user intervening if they upgrade.

Holding off on re-casing the names because I think I'm going to restructure them - they follow the JavaScript plugin naming by and large, which I never really liked. When I do that, I'll probably revisit the above.

@Pablissimo
Copy link
Owner

(The above changes would be probably what tip us into v1.0 territory really - tidy up, obvious version bump to indicate the changed behaviour, refresh all the docs and put some better examples of usage in)

Pablissimo added a commit that referenced this issue Jan 18, 2017
Fix issue #75: adds support for two new options:

- ```sonar.ts.tslintprojectpath``` - set to 'tsconfig.json' or similar, the path to your TypeScript configuration file describing what files to compile and lint
- ```sonar.ts.tslinttypecheck``` - true/false, defaults to false - if true, requests ```tslint``` perform a type-check too, which allows certain rules requiring type information to work
@Pablissimo
Copy link
Owner

Released as v0.99

@NikitaEgorov
Copy link
Contributor Author

@Pablissimo
Why did you commented this part of code?

/*,
    @Property(
        key = TypeScriptPlugin.SETTING_TS_LINT_TYPECHECK,
        defaultValue = "false",
        type = PropertyType.BOOLEAN,
        name = "Forces tslint to run a type-check",
        description = "Equivalent to --type-check tslint argument - requires tslintconfigpath also set",
        project = true,
        global = false
    ),
    @Property(
        key = TypeScriptPlugin.SETTING_TS_LINT_TSCONFIG_PATH,
        defaultValue = "",
        type = PropertyType.STRING,
        name = "Path to tsconfig.json file, if required",
        description = "Required if tslinttypecheck parameter specified, the path to the tsconfig.json file that describes the files to lint and build",
        project = true,
        global = false
    )*/

@Pablissimo
Copy link
Owner

Pablissimo commented Jan 19, 2017 via email

@NikitaEgorov
Copy link
Contributor Author

NikitaEgorov commented Jan 19, 2017

@Pablissimo
I update plugin for Sonar 6.2

2017-01-19T09:07:06.9993123Z ##[error]09:07:06.887 WARN: TsLint reported issues against a file that wasn't sent to it - will be ignored: ****.ts
2017-01-19T09:07:07.0002889Z ##[error]09:07:06.887 WARN: TsLint reported issues against a file that wasn't sent to it - will be ignored: ****.ts
2017-01-19T09:07:07.0002889Z ##[error]09:07:06.887 WARN: TsLint reported issues against a file that wasn't sent to it - will be ignored: ****.ts
pathAdjusted: C:/TfsAgent/_work/5/s/dev/src/system/widgets/confirm.ts
TsLint reported issues against a file that wasn't sent to it - will be ignored: c:/TfsAgent/_work/5/s/dev/src/system/widgets/confirm.ts

I've got it

C:/
c:/

@Pablissimo
Copy link
Owner

Pablissimo commented Jan 19, 2017 via email

@NikitaEgorov
Copy link
Contributor Author

@Pablissimo
Now I change TsLintSensor.execute

   String pathAdjusted = file.absolutePath().replace('\\', '/').toLowerCase(); //!!!!
            paths.add(pathAdjusted);

...

 for (String filePath : issues.keySet()) {

            List<TsLintIssue> batchIssues = issues.get(filePath);

            if (batchIssues == null || batchIssues.size() == 0) {
                continue;
            }

            String lowerFilePath = filePath.toLowerCase(); //!!!!

            if (!fileMap.containsKey(lowerFilePath)) { //!!!!
                LOG.warn("TsLint reported issues against a file that wasn't sent to it - will be ignored: " + lowerFilePath);
                continue;
            }

            InputFile file = fileMap.get(lowerFilePath); //!!!!

Because

 if (!fileMap.containsKey(lowerFilePath)) { //!!!!
                LOG.warn("TsLint reported issues against a file that wasn't sent to it - will be ignored: " + lowerFilePath);
                continue; //!!!!!!!!!!!!!!!!!!
            }

@Pablissimo
Copy link
Owner

Pablissimo commented Jan 19, 2017 via email

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

No branches or pull requests

4 participants