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

Show unused public properties and methods #29293

Open
jabacchetta opened this issue Jan 5, 2019 · 40 comments
Open

Show unused public properties and methods #29293

jabacchetta opened this issue Jan 5, 2019 · 40 comments
Labels
In Discussion Not yet reached consensus Suggestion An idea for TypeScript

Comments

@jabacchetta
Copy link

jabacchetta commented Jan 5, 2019

It would be nice to have unused detection expanded, capable of detecting unused methods, exports, etc.

Code Example

Shape.js

class Shape {
    constructor() {
        this.color = 'red';
    }

    colorLog() {
        console.log(this.color);
    }
}

export default Shape;

Circle.js

import Shape from "./Shape";

class Circle extends Shape {
  constructor() {
    super();

    // FIXME: description should show as unused in VSCode, as it does in WebStorm.
    this.description = 'A circle shape';
  }

  // FIXME: circleLog should show as unused in VSCode, as it does in WebStorm.
  circleLog() {
    // NOTE: both VSCode and WebStorm have detected an unused variable.
    const num = 2;

    // NOTE: colorLog is being used, so no unused code errors in Shape.js file.
    super.colorLog();
  }
}

// FIXME: export should show as unused in VSCode, as it does in WebStorm.
export default Circle;

VSCode Screen

screen shot 2019-01-04 at 9 23 48 pm

WebStorm Screen

screen shot 2019-01-04 at 9 23 07 pm

@vscodebot vscodebot bot assigned mjbvz Jan 5, 2019
@mjbvz mjbvz transferred this issue from microsoft/vscode Jan 7, 2019
@microsoft microsoft deleted a comment from vscodebot bot Jan 7, 2019
@mjbvz
Copy link
Contributor

mjbvz commented Jan 7, 2019

We currently assume that exported members form a public API and therefore are always used. We could try to detect that a certain set of files are an API entrypoints, and then mark unused internal exports. Not sure if there are any issue already tracking this

@mjbvz mjbvz removed their assignment Jan 7, 2019
@weswigham weswigham added Suggestion An idea for TypeScript In Discussion Not yet reached consensus labels Jan 8, 2019
@mjbvz mjbvz changed the title Expand unused detection Show unused public properties and methods Feb 20, 2019
@auchenberg
Copy link

Customer demand for this feature mentioned in https://dev.to/mokkapps/why-i-switched-from-visual-studio-code-to-jetbrains-webstorm-939.

@zpdDG4gta8XKpMCd
Copy link

zpdDG4gta8XKpMCd commented Mar 1, 2019

what about exported but unused

  • modules
  • classes
  • types
  • interfaces/props
  • members (enum | union)
  • functions
  • constants/variables?

@fatihturgut
Copy link

is there any news about this issue?

@jp7837
Copy link

jp7837 commented Oct 16, 2019

Update: much faster after adding dummy getProjectVersion implementation assuming the project doesn't change during execution.

My naive solution below. It's pretty slow even though it should only run a full compilation once. I'll post updates if I improve it.

const fs = require("fs");
const ts = require("typescript");
const path = require("path");

// Copied from https://github.com/microsoft/TypeScript/wiki/Using-the-Compiler-API#incremental-build-support-using-the-language-services
function getLanguageService(rootFileNames, options) {
	const files = {};

	// initialize the list of files
	rootFileNames.forEach(fileName => {
		files[fileName] = { version: 0 };
	});

	// Create the language service host to allow the LS to communicate with the host
	const servicesHost = {
		getScriptFileNames: () => rootFileNames,
		getScriptVersion: fileName => files[fileName] && files[fileName].version.toString(),
		getScriptSnapshot: fileName => {
			if (!fs.existsSync(fileName)) {
				return undefined;
			}

			return ts.ScriptSnapshot.fromString(fs.readFileSync(fileName).toString());
		},
		getCurrentDirectory: () => process.cwd(),
		getCompilationSettings: () => options,
		getDefaultLibFileName: options => ts.getDefaultLibFilePath(options),
		getProjectVersion: () => 1,
		fileExists: ts.sys.fileExists,
		readFile: ts.sys.readFile,
		readDirectory: ts.sys.readDirectory,
	};

	// Create the language service files
	const services = ts.createLanguageService(servicesHost, ts.createDocumentRegistry());

	return services;
}

const tsconfigPath = "tsconfig.gen.json";
const basePath = path.resolve(path.dirname(tsconfigPath));
const parseJsonResult = ts.parseConfigFileTextToJson(tsconfigPath, fs.readFileSync(tsconfigPath, { encoding: "utf8" }));
const tsConfig = ts.parseJsonConfigFileContent(parseJsonResult.config, ts.sys, basePath);
const services = getLanguageService(tsConfig.fileNames, tsConfig.options);

// For each non-typings file
tsConfig.fileNames
	.filter(f => !f.endsWith(".d.ts"))
	.forEach(file => {
		const source = ts.createSourceFile(file, fs.readFileSync(file, { encoding: "utf8" }));
		ts.forEachChild(source, node => {
			if (ts.isClassDeclaration(node)) {
				// For each class member
				node.members.forEach(member => {
					// If member is marked as public or protected and not a constructor
					if (
						(ts.getCombinedModifierFlags(member) & ts.ModifierFlags.Public ||
							ts.getCombinedModifierFlags(member) & ts.ModifierFlags.Protected) &&
						member.kind !== ts.SyntaxKind.Constructor
					) {
						const references = services.findReferences(file, member.name.pos + 1);
						// Fail if every reference is a definition and not in a typings file
						if (
							references.every(
								reference =>
									reference.references.length === 1 &&
									reference.references[0].isDefinition &&
									!reference.definition.fileName.endsWith(".d.ts")
							)
						) {
							console.error(`File: ${file} , Member: ${member.name.text}`);
						}
					}
				});
			}
		});
	});

@alanhe421
Copy link

I was wondering if we could use custom rules to detect and fix this kind of problem

@Bogidon
Copy link

Bogidon commented Mar 25, 2020

I would like this because using top-level exports, and destructuring imports can clutter your files quite a lot. E.g. I would like to make a Util class I can use the default import on, without worrying about forgetting to delete unused functions a year down the road.

Or would namespaces be a solution to this problem? (does unused code detection work on namespaces?)

@iulianraduat
Copy link

I am also interested to remove the kind of dead code mentioned by zpdDG4gta8XKpMCd .

Maybe Patricio Zavolinsky (pzavolinsky), creator of https://www.npmjs.com/package/ts-unused-exports, can help the Visual Code team to implement it?

@SigmaOutdoors
Copy link

Another vote for this feature, first priority IMO would be some kind of indication for unused variables - grey/red squiggles, don't care much.

@delucca
Copy link

delucca commented Jul 5, 2021

No updates about this feature yet? This is present is most modern IDEs. It is a shame that VSCode doesn't support it.

@bencun
Copy link

bencun commented Jul 6, 2021

This would be tremendously useful. Detecting and eliminating unused public methods, exports, etc. is something I feel Code is really missing at this point. Any updates on this feature?
I do understand it's a difficult task to undertake at this point, though.

@iulianraduat
Copy link

Until there is an official version, I created my own extension to find unused exports.
If you want to give it a try please install it from https://marketplace.visualstudio.com/items?itemName=iulian-radu-at.find-unused-exports
There are already requests to extend its functionality so if you feel the same, any help in this direction is much appreciated.

@ivan-myronov-windmill
Copy link

ivan-myronov-windmill commented Jan 19, 2022

Update: much faster after adding dummy getProjectVersion implementation assuming the project doesn't change during execution.

My naive solution below. It's pretty slow even though it should only run a full compilation once. I'll post updates if I improve it.

const fs = require("fs");
const ts = require("typescript");
const path = require("path");

// Copied from https://github.com/microsoft/TypeScript/wiki/Using-the-Compiler-API#incremental-build-support-using-the-language-services
function getLanguageService(rootFileNames, options) {
	const files = {};

	// initialize the list of files
	rootFileNames.forEach(fileName => {
		files[fileName] = { version: 0 };
	});

	// Create the language service host to allow the LS to communicate with the host
	const servicesHost = {
		getScriptFileNames: () => rootFileNames,
		getScriptVersion: fileName => files[fileName] && files[fileName].version.toString(),
		getScriptSnapshot: fileName => {
			if (!fs.existsSync(fileName)) {
				return undefined;
			}

			return ts.ScriptSnapshot.fromString(fs.readFileSync(fileName).toString());
		},
		getCurrentDirectory: () => process.cwd(),
		getCompilationSettings: () => options,
		getDefaultLibFileName: options => ts.getDefaultLibFilePath(options),
		getProjectVersion: () => 1,
		fileExists: ts.sys.fileExists,
		readFile: ts.sys.readFile,
		readDirectory: ts.sys.readDirectory,
	};

	// Create the language service files
	const services = ts.createLanguageService(servicesHost, ts.createDocumentRegistry());

	return services;
}

const tsconfigPath = "tsconfig.gen.json";
const basePath = path.resolve(path.dirname(tsconfigPath));
const parseJsonResult = ts.parseConfigFileTextToJson(tsconfigPath, fs.readFileSync(tsconfigPath, { encoding: "utf8" }));
const tsConfig = ts.parseJsonConfigFileContent(parseJsonResult.config, ts.sys, basePath);
const services = getLanguageService(tsConfig.fileNames, tsConfig.options);

// For each non-typings file
tsConfig.fileNames
	.filter(f => !f.endsWith(".d.ts"))
	.forEach(file => {
		const source = ts.createSourceFile(file, fs.readFileSync(file, { encoding: "utf8" }));
		ts.forEachChild(source, node => {
			if (ts.isClassDeclaration(node)) {
				// For each class member
				node.members.forEach(member => {
					// If member is marked as public or protected and not a constructor
					if (
						(ts.getCombinedModifierFlags(member) & ts.ModifierFlags.Public ||
							ts.getCombinedModifierFlags(member) & ts.ModifierFlags.Protected) &&
						member.kind !== ts.SyntaxKind.Constructor
					) {
						const references = services.findReferences(file, member.name.pos + 1);
						// Fail if every reference is a definition and not in a typings file
						if (
							references.every(
								reference =>
									reference.references.length === 1 &&
									reference.references[0].isDefinition &&
									!reference.definition.fileName.endsWith(".d.ts")
							)
						) {
							console.error(`File: ${file} , Member: ${member.name.text}`);
						}
					}
				});
			}
		});
	});

Any ideas how do the same for composed types transformed with Pick?

For example:

export type SessionStreamConfigurationFragment = (
  { __typename?: 'Session' }
  & Pick<Session, 'showRecording'>
  & { oneWayStreamConfiguration?: Maybe<(
    { __typename?: 'OneWayStreamConfiguration' }
    & OneWayStreamConfigurationFragment
  )> }
);

@jasonwilliams
Copy link

jasonwilliams commented Jun 10, 2022

I had a similar query about unused exports which ive raised here: #30517

@jpike88
Copy link

jpike88 commented Jul 5, 2022

this would be huge. I have an angular project with loads of classes with static methods that are probably not used anymore. It's too much to audit method by method.

@douglas-pires
Copy link

It would be very interesting to see this feature. I'm currently working on NestJS projects that would benefit from something like this.

@gethari
Copy link

gethari commented Dec 11, 2022

Ah having a huge #Angular repo, would definitely need this in vscode 🥺

@gethari
Copy link

gethari commented Dec 11, 2022

this would be huge. I have an angular project with loads of classes with static methods that are probably not used anymore. It's too much to audit method by method.

Did you get any solution by any chance ?

@TroelsWibergJensen

This comment has been minimized.

1 similar comment
@vincentole

This comment has been minimized.

@chris-dallaire

This comment was marked as off-topic.

@chris-dallaire
Copy link

chris-dallaire commented Apr 18, 2023

One of my engineering peers keeps pointing out stupid errors I'm making in PR reviews because he's got way better static code analysis tools in WebStorm than I have in VSCode which I've been a loyal user of for years.

This is embarrassing and making me look like an even worse programmer than I already am - please fix it!

@osenvosem

This comment was marked as off-topic.

@Shagon1k

This comment has been minimized.

@metalsadman

This comment has been minimized.

@gethari

This comment has been minimized.

@eternal-eager-beaver

This comment was marked as off-topic.

@eliasrmz01

This comment has been minimized.

@jpike88
Copy link

jpike88 commented Oct 25, 2023

We have lots of dead public properties on our shared classes, it's painstaking to audit them, I have to use 'find all references' for each property in vscode.

@tlcpack

This comment was marked as off-topic.

@dave0688

This comment has been minimized.

@muchisx

This comment has been minimized.

@archubbuck

This comment has been minimized.

@RyanCavanaugh
Copy link
Member

RyanCavanaugh commented May 1, 2024

knip is a pretty good solution to this.

Please refrain from posting low-value comments in this thread.

@jpike88
Copy link

jpike88 commented May 1, 2024

Would knip find an unused public function on a component though? Those kinds of things are what I'm really looking to root out.

@RyanCavanaugh
Copy link
Member

I'm not sure, but it's not even clear that TypeScript could do a good job of that. If you have something like

export class Foo {
  bar() { }
  baz() { }
}

interface HasBarAndBaz {
  bar(): void;
  baz(): void;
}

const p: HasBarAndBaz = new Foo();
p.baz();

It's not clear how you really figure out that Foo#baz is called while not mistakenly thinking Foo#bar isn't unused. Detecting unused properties vs detecting unused exports is really two different things.

@Papooch
Copy link

Papooch commented May 14, 2024

It's not clear how you really figure out [...]

In my opinion it is very clear that the desired behavior is to do what webstorm already does.

I'm torn between whether this should be a feature of the language itself or a feature of a separate tool, but in the end, all I really care about is that VSCode can tell me which public methods are unused by the rest of the code (for library code that's not consumed within the same codebase, it is also a good indicator of spotting untested methods)

@RyanCavanaugh
Copy link
Member

I don't know what Webstorm's algorithm is or what its false positives/negatives are. Can you go into more detail on it?

@sixtyfive
Copy link

So it's 2024 and I had to touch Typescript for the first time today and within a few minutes wished for there to be a visual indicator for an unused method and landed here, in this Issue from 2019. Am I understanding correctly that even after 5 years this is still not possible and people are still being recommended Webstorm instead?

@dave0688
Copy link

dave0688 commented Jun 6, 2024

You are absolutely correct. This is a shame!
VS Code is such a nice IDE, but there are some features which are just better in Webstorm...
I'm thinking of switching, since in quite some issues there is no movement.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
In Discussion Not yet reached consensus Suggestion An idea for TypeScript
Projects
None yet
Development

No branches or pull requests