Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Keep track of why files are in the program #40011

Merged
merged 34 commits into from
Dec 9, 2020
Merged

Keep track of why files are in the program #40011

merged 34 commits into from
Dec 9, 2020

Conversation

sheetalkamat
Copy link
Member

@sheetalkamat sheetalkamat commented Aug 12, 2020

  1. Instead of keeping reference file reasons tracking, we also keep track of all reasons file is included in the program.
  2. The configFileSpecs are stored on tsconfigFile so we dont have to pass them around any more.
  3. The reasons are used to write explanations for files when new option --explainFiles is specified and program can use it too (earlier program wasnt aware of these)
  4. They are also used to report errors about the file eg its not in rootDir or doesn't match casing etc
  5. Because it is used to report errors while file processing is going on and program is still under construction, we store the metadata for errors instead of converting them in diagnostic right away. This is so we can reuse this data when program is completely reused and still give error locations at correct position.(Because the related information location can change without change but the diagnostic can be in different file)

Fixes #33515

@typescript-bot typescript-bot added Author: Team For Uncommitted Bug PR for untriaged, rejected, closed or missing bug labels Aug 12, 2020
@sheetalkamat
Copy link
Member Author

@typescript-bot pack this

@typescript-bot
Copy link
Collaborator

typescript-bot commented Dec 3, 2020

Heya @sheetalkamat, I've started to run the tarball bundle task on this PR at 3a4613e. You can monitor the build here.

@sheetalkamat
Copy link
Member Author

@typescript-bot pack this

@typescript-bot
Copy link
Collaborator

typescript-bot commented Dec 3, 2020

Heya @sheetalkamat, I've started to run the tarball bundle task on this PR at 87ca884. You can monitor the build here.

@typescript-bot
Copy link
Collaborator

typescript-bot commented Dec 3, 2020

Hey @sheetalkamat, I've packed this into an installable tgz. You can install it for testing by referencing it in your package.json like so:

{
    "devDependencies": {
        "typescript": "https://typescript.visualstudio.com/cf7ac146-d525-443c-b23c-0d58337efebc/_apis/build/builds/90322/artifacts?artifactName=tgz&fileId=41E4437F40EC197DEB7906352B4A844350F4EE385720380498BA5240C44E9B9602&fileName=/typescript-4.2.0-insiders.20201203.tgz"
    }
}

and then running npm install.


There is also a playground for this build.

@RyanCavanaugh
Copy link
Member

Trying on a local build just to see how it feels

D:\github\TypeScript\src\services>tsc --explainFiles
C:/Users/Ryan/AppData/Roaming/npm/node_modules/typescript/lib/lib.es5.d.ts
  Library 'lib.es5.d.ts' specified in compilerOptions
C:/Users/Ryan/AppData/Roaming/npm/node_modules/typescript/lib/lib.es2015.generator.d.ts
  Library 'lib.es2015.generator.d.ts' specified in compilerOptions
C:/Users/Ryan/AppData/Roaming/npm/node_modules/typescript/lib/lib.es2015.iterable.d.ts
  Library 'lib.es2015.iterable.d.ts' specified in compilerOptions
  Library referenced via 'es2015.iterable' from file 'C:/Users/Ryan/AppData/Roaming/npm/node_modules/typescript/lib/lib.es2015.generator.d.ts'
C:/Users/Ryan/AppData/Roaming/npm/node_modules/typescript/lib/lib.es2015.symbol.d.ts
  Library referenced via 'es2015.symbol' from file 'C:/Users/Ryan/AppData/Roaming/npm/node_modules/typescript/lib/lib.es2015.iterable.d.ts'
../../built/local/shims.d.ts
  Output from referenced project '../shims/tsconfig.json' included because '--outFile' specified
../../node_modules/@types/microsoft__typescript-etw/index.d.ts
  Imported via "@microsoft/typescript-etw" from file '../../built/local/compiler.d.ts' with packageId '@types/microsoft__typescript-etw/index.d.ts@0.1.1'
../../built/local/compiler.d.ts
  Output from referenced project '../compiler/tsconfig.json' included because '--outFile' specified
../../built/local/jsTyping.d.ts
  Output from referenced project '../jsTyping/tsconfig.json' included because '--outFile' specified
types.ts
  Part of 'files' list in tsconfig.json
utilities.ts
  Part of 'files' list in tsconfig.json
classifier.ts
  Part of 'files' list in tsconfig.json
stringCompletions.ts
  Part of 'files' list in tsconfig.json
completions.ts
  Part of 'files' list in tsconfig.json
documentHighlights.ts
  Part of 'files' list in tsconfig.json
documentRegistry.ts
  Part of 'files' list in tsconfig.json
importTracker.ts
  Part of 'files' list in tsconfig.json
findAllReferences.ts
  Part of 'files' list in tsconfig.json
callHierarchy.ts
  Part of 'files' list in tsconfig.json
getEditsForFileRename.ts
  Part of 'files' list in tsconfig.json
goToDefinition.ts
  Part of 'files' list in tsconfig.json
jsDoc.ts
  Part of 'files' list in tsconfig.json
navigateTo.ts
  Part of 'files' list in tsconfig.json
(many more, truncated)
@DanielRosenwasser
Copy link
Member

DanielRosenwasser commented Dec 4, 2020

I like the idea, I would prefer different output. Maybe one line per file for easier regex/grepping?

fileName.ts: Explanation goes here.
@RyanCavanaugh
Copy link
Member

I would prefer different output.

Files can have multiple reasons for being in the program - how do you think that should be handled as a single-line format?

@DanielRosenwasser
Copy link
Member

DanielRosenwasser commented Dec 4, 2020

Huh, didn't catch that from the example 😅- I would say to repeat the file name on both lines. Would that be reasonable?

@sheetalkamat
Copy link
Member Author

Ready for review.
Quick question @DanielRosenwasser: do you want me to rename the flag to explainInputFiles ?

PR Backlog automation moved this from Needs review to Needs merge Dec 8, 2020
@sheetalkamat sheetalkamat merged commit 2eca17d into master Dec 9, 2020
PR Backlog automation moved this from Needs merge to Done Dec 9, 2020
@sheetalkamat sheetalkamat deleted the explainFiles branch December 9, 2020 00:10
@thetutlage
Copy link

I see that the configFileSpecs has been removed from the output of getParsedCommandLineOfConfigFile. What's is alternative to get access to it?

@sheetalkamat
Copy link
Member Author

The configFileSpecs are stored on tsconfigFile so we dont have to pass them around any more.

@thetutlage
Copy link

I read that, but looking for the ways to access them. Is there is any new API for the same?

@sheetalkamat
Copy link
Member Author

It was always internal (meaning subject to change). We do not plan to expose that through API.

@pelikhan
Copy link
Member

Any easily parseable format would be nice JSON or directly graphviz style files; would allow for visualizations and tooling.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Author: Team For Uncommitted Bug PR for untriaged, rejected, closed or missing bug
6 participants