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

Feature Request: Typescript dead code cleanup #16939

Closed
akashperfect opened this issue Jul 5, 2017 · 4 comments
Closed

Feature Request: Typescript dead code cleanup #16939

akashperfect opened this issue Jul 5, 2017 · 4 comments
Labels
Out of Scope This idea sits outside of the TypeScript language design constraints Suggestion An idea for TypeScript

Comments

@akashperfect
Copy link

akashperfect commented Jul 5, 2017

TypeScript Version: 2.4.0 / nightly (2.5.0-dev.201xxxxx)

Problem Statement
Unlike C# there is no utility for typescript to clean up the dead code. Here by dead code I mean the functions which are nowhere referenced or called anymore.

Programmers are mainly affected by this issue, like as an example I was cleaning up the old code after moving to different framework in our project. I saw it extremely cumbersome to remove existing files as just deleting them would cause some other classes public function and components to be left dangling with no references what so ever. Although I agree the compiler is optimized to include only used Js files in the bundle but for code search and maintainability it’s a major work.

Expected behavior:
Best way would be to delete all the obsolete files and run the utility to get all the dead functions formed as part of the files deletion. This will make everyone’s life much simpler as they wouldn’t have to be bothered about un-used functions.

Actual behavior:
Need to manually look for all the imports and need to delete those if not referenced anywhere else.

What didn't worked
From github issue someone mentions using noUnusedParameters and noUnusedLocals but these are only for if local variables or if any parameter is used or not. But there is no way to determine that in the entire project what all functions are never called.
Using static analysis from Visual Studio also didn't helped in this matter.

Proposed Change
Just a high level idea
Compiler would parse through all the code and maintain reference, so right after parsing(output from parser.ts) we can check what all functions are not referenced from any other part of the code in the entire project/solution. We can mark them as dead and good to be removed.
This could come either as part of an extension to run separately or a static analysis rule to be included in the build.

Example
Consider 2 files having some code of this type
file1.ts

export class file1 {
   public foo() {
      return;
   }
   // other functions
}

file.ts

import { file1 } from "./file1";
export class file {
   public bar() {
      var test = new file1();
      test.foo();
   }
}

Now suppose I no longer need file.ts, I will delete that and now since the function foo() was used only by function bar(), I should ideally delete foo() also from file1.ts. Now my point being that there is no such utility which achieves this today.

@jklmli
Copy link

jklmli commented Jul 5, 2017

tslint supports this.

@akashperfect
Copy link
Author

akashperfect commented Jul 5, 2017

@jiaweihli According to the documentation this is Similar to tsc’s –noUnusedParameters and –noUnusedLocals options, which are for local variables and un-used imports only. However, the problem I am stating is different where we use public function of a different class in our class. Now, if we delete our class and if the function from other class was written specifically for our's we should ideally delete that too.
Also, I have added an example you may refer to that. Please let me know if you have come across something which achieves this.

@kitsonk
Copy link
Contributor

kitsonk commented Jul 5, 2017

@jiaweihli no, that will only track unused variables, including imports... not if the file is part of a dependency graph of a project...

I still don't think this should be part of TypeScript. Specifically non-goal:

  1. Provide an end-to-end build pipeline. Instead, make the system extensible so that external tools can use the compiler for more complex build workflows.

TypeScript isn't the best tool for building a dependency graph of files. Something like webpack or rollup are much better tools and they provide all the sort of information of what is and what is not depended upon. TypeScript is a transpiler, taking one form of code and outputting it in another form of code. It resolves dependencies to the level it has the information it needs to properly transpile its code and provide intelligence on likely static identifiable code errors. We should use the tools that are best suited for the task at hand.

@mhegazy
Copy link
Contributor

mhegazy commented Aug 22, 2017

Doing full program dead-code elimination can get hairy quickly; for instance, what happens when you have modules, are these modules all visible, or some of them are internal implementation details? if so then how do you communicate to the compiler which is which? a set of entry-point modules possibly, and let the compiler figure out the graph? what about framework-specific features, e.g. Angular2 decorators will set some properties for you, the compiler has no information about these ones, are they errors? may be some sort of an exception list? what about virtual methods that you are supposed to implement, e.g. React's shouldComponentUpdate or render, these are not used in your project, should these be flagged? maybe? maybe another exclusion list for methods/properties that are never used. should we do it on class level? or module level... etc..

the other interesting point is doing this analysis is expensive, and maintaining it in an editor scenario would put a tax for the general case, without much value added. i would say these can be done in a standalone tool that you run it once, possibly after a large refactoring, go through the errors and fix ones you deem correct.

As @kitsonk noted, this is currently outside the scope of the TS project.

@mhegazy mhegazy added Out of Scope This idea sits outside of the TypeScript language design constraints Suggestion An idea for TypeScript labels Aug 22, 2017
@mhegazy mhegazy closed this as completed Aug 22, 2017
@microsoft microsoft locked and limited conversation to collaborators Jun 14, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Out of Scope This idea sits outside of the TypeScript language design constraints Suggestion An idea for TypeScript
Projects
None yet
Development

No branches or pull requests

4 participants