Skip to content

Warn if unused top-level function is not included is signature file. - #14567

Merged
vzarytovskii merged 12 commits into
dotnet:mainfrom
nojaf:unused-function-signature-file
Jan 20, 2023
Merged

Warn if unused top-level function is not included is signature file.#14567
vzarytovskii merged 12 commits into
dotnet:mainfrom
nojaf:unused-function-signature-file

Conversation

@nojaf

@nojafnojaf commented Jan 9, 2023•
edited
Loading

Copy link
Copy Markdown
Contributor

Hello, I would like to warn the user when a public function in an implementation file is not used and is not part of the signature file.

The implementation could improve, but I think the idea holds.
Let me know how I can properly check for this.

@nojaf
nojaf requested a review from a team as a code ownerJanuary 9, 2023 16:43
@nojafnojaf changed the title Warn is unused toplevel function is not included is signature file.Warn if unused top-level function is not included is signature file.Jan 9, 2023
auduchinok
auduchinok reviewed Jan 9, 2023
View reviewed changes
Comment threadsrc/Compiler/Checking/PostInferenceChecks.fs Outdated
@nojaf
nojafforce-pushed the unused-function-signature-file branch from ef10779 to 34f45a8CompareJanuary 11, 2023 11:50
@nojafnojaf mentioned this pull request Jan 11, 2023
Merged
T-Gro
T-Gro reviewed Jan 11, 2023
View reviewed changes
Comment threadsrc/Compiler/Checking/PostInferenceChecks.fs Outdated
auduchinok
auduchinok reviewed Jan 11, 2023
View reviewed changes
Comment threadsrc/Compiler/Checking/PostInferenceChecks.fs Outdated
@nojaf
nojafforce-pushed the unused-function-signature-file branch from 5ec4a7d to 6e3aa92CompareJanuary 16, 2023 12:40
@nojaf

nojaf commented Jan 17, 2023

Copy link
Copy Markdown
ContributorAuthor

Ready for review

abonie
abonie approved these changes Jan 17, 2023
View reviewed changes
Comment threadtests/FSharp.Compiler.ComponentTests/CompilerOptions/fsc/warnon/warnon.fs
@nojaf
nojaf requested a review from T-GroJanuary 19, 2023 07:42
edgarfgp
edgarfgp approved these changes Jan 19, 2023
View reviewed changes

@edgarfgpedgarfgp left a comment •
edited
Loading

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This a needed warning when using signature files :) . Thanks

psfinaki
psfinaki approved these changes Jan 19, 2023
View reviewed changes

@psfinakipsfinaki left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Good stuff. It's like - "we should have had this 10 years ago". Thanks :)

auduchinok
auduchinok reviewed Jan 19, 2023
View reviewed changes
Comment threadsrc/Compiler/Checking/PostInferenceChecks.fs
@vzarytovskii
vzarytovskii enabled auto-merge (squash) January 20, 2023 12:20
@vzarytovskii
vzarytovskii merged commit a4ec896 into dotnet:mainJan 20, 2023
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@vzarytovskiivzarytovskiivzarytovskii approved these changes
@abonieabonieabonie approved these changes
@T-GroT-GroAwaiting requested review from T-Gro
+3 more reviewers
@auduchinokauduchinokauduchinok left review comments
@psfinakipsfinakipsfinaki approved these changes
@edgarfgpedgarfgpedgarfgp approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

F# Compiler and Tooling
Archived in project

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

7 participants

@nojaf@vzarytovskii@auduchinok@psfinaki@abonie@edgarfgp@T-Gro