Skip to content

Conversation

@xiemaisi
Copy link

Every once in a while we encounter projects using some custom file extension for files that we could in principle extract, but since the extractor doesn't know about the extension the files are skipped.

To handle this, the legacy extractor has a --file-type option that one can use to specify a file type to use for all files in that particular extraction. So far, AutoBuild has nothing of the sort.

This PR proposes to introduce an environment variable LGTM_INDEX_FILETYPES to allow a similar customisation. In the fullness of time, this variable would be set through lgtm.yml in the usual way, but for now it is undocumented and for internal use only.

Specifically, LGTM_INDEX_FILETYPES is a newline-separated list of ".extension:filetype" pairs, specifying that files with the given .extension should be extracted as type filetype, where filetype is one of js, html, json, typescript or yaml.

For example, .jsm:js causes all .jsm files to be extracted as JavaScript.

This can also be used to override default file types: for example, by specifying .js:typescript all JavaScript files will be extracted as TypeScript.

Max Schaefer added 2 commits February 27, 2019 11:54
Every once in a while we encounter projects using some custom file extension for files that we could in principle extract, but since the extractor doesn't know about the extension the files are skipped.

To handle this, the legacy extractor has a `--file-type` option that one can use to specify a file type to use for all files in that particular extraction. So far, `AutoBuild` has nothing of the sort.

This PR proposes to introduce an environment variable `LGTM_INDEX_FILETYPES` to allow a similar customisation. In the fullness of time, this variable would be set through `lgtm.yml` in the usual way, but for now it is undocumented and for internal use only.

Specifically, `LGTM_INDEX_FILETYPES` is a newline-separated list of ".extension:filetype" pairs, specifying that files with the given `.extension` should be extracted as type `filetype`, where
`filetype` is one of `js`, `html`, `json`, `typescript` or `yaml`.

For example, `.jsm:js` causes all `.jsm` files to be extracted as JavaScript.

This can also be used to override default file types: for example, by specifying `.js:typescript` all JavaScript files will be extracted as TypeScript.
@xiemaisi xiemaisi added JS WIP This is a work-in-progress, do not merge yet! labels Feb 27, 2019
@xiemaisi xiemaisi requested a review from a team as a code owner February 27, 2019 12:13
@xiemaisi
Copy link
Author

I'll do a brief evaluation, so WIP for now.

Copy link
Contributor

@asger-semmle asger-semmle left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@xiemaisi
Copy link
Author

A test on gecko-dev with LGTM_INDEX_FILETYPES=.jsm:js showed the number of extracted files rising from 137348 to 138185, with a bunch of new alerts in .jsm files and a few fixed alerts in .js files, the latter primarily due to a few global functions and property accessors defined in .jsm files that cause conservative FP avoidance conditions to kick in. Overall analysis time increases by about 4.5%.

All of this is expected, so I think this is good to go.

I've opened #1003 to fix the language test failure, which seems unrelated to this PR.

@xiemaisi xiemaisi removed the WIP This is a work-in-progress, do not merge yet! label Feb 28, 2019
@xiemaisi xiemaisi added this to the 1.20 milestone Feb 28, 2019
@xiemaisi
Copy link
Author

@asger-semmle, do you want me to rebase (and hence trigger a re-run of the tests)? The only language test failure is from the problem fixed in #1003, and the pr checks complain about tabs-vs-spaces which I'm going to address by auto-formatting the extractor Any Day Now.

@asger-semmle asger-semmle merged commit 5478e0d into github:master Feb 28, 2019
@xiemaisi xiemaisi deleted the js/autobuild-file-types branch March 13, 2019 15:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants