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

[display] unused pattern variables should be marked as unused #7282

Closed
Tracked by #11707
Gama11 opened this issue Jul 18, 2018 · 11 comments
Closed
Tracked by #11707

[display] unused pattern variables should be marked as unused #7282

Gama11 opened this issue Jul 18, 2018 · 11 comments
Assignees
Labels
feature-ide IDE / Editor support
Milestone

Comments

@Gama11
Copy link
Member

Gama11 commented Jul 18, 2018

Similar to #7267 - value should have a "this code has no effect" diagnostic so vshaxe can gray it out.

import haxe.ds.Option;

class Main {
	public static function main() {
		switch ((null:Option<Int>)) {
			case Some(value):
			case None:
		}
	}
}
@markknol markknol added the feature-ide IDE / Editor support label Jul 26, 2018
@Simn
Copy link
Member

Simn commented Aug 23, 2018

I don't know how I feel about that. It might be somewhat impractical, especially if you consider code-generation for switch expressions. It's also not exactly true to say that the code has no effect: It's actually required to match the structure and the only way to avoid warnings would be to use _ everywhere.

@Gama11
Copy link
Member Author

Gama11 commented Aug 23, 2018

The idea here is that we need the diagnostic to be able to gray the variables out in vshaxe, like we do for other unused code. You wouldn't even normally see the "this code has no effect" message, and that could be something else.

@Gama11
Copy link
Member Author

Gama11 commented Aug 23, 2018

We would probably also want to offer a "replace with _" code action for these, so it should probably be a different message / type, so we can properly distuingish it.

@Gama11
Copy link
Member Author

Gama11 commented Aug 25, 2018

Oh, and also note that we don't present any of the "unused" diagnostics as actual warnings anymore (so they also don't show up in the problems view). That would show green underlines in addition to the grayed-out-effect, which kind of destroys the subtlety.

https://github.com/vshaxe/haxe-languageserver/blob/38553fd756de2a194e53647348ae93adddf68256/src/haxeLanguageServer/features/DiagnosticsManager.hx#L152-L154

@Simn Simn added this to the Design milestone Dec 12, 2018
@Simn Simn self-assigned this Dec 12, 2018
@Gama11
Copy link
Member Author

Gama11 commented Mar 8, 2019

Huh, was this accidentally fixed somehow? Now seems to be working:

I guess I'll add a test and close this.

@Gama11
Copy link
Member Author

Gama11 commented Mar 8, 2019

Oh, this is reported as DKRemovableCode, and the range on that is pretty off..

@Simn
Copy link
Member

Simn commented Jul 2, 2019

This is easy to fix but requires syncing with vshaxe which I currently cannot compile.

@Simn
Copy link
Member

Simn commented Nov 22, 2023

This (alongside vshaxe/haxe-language-server@0f3418b) now gives a more useful code action:

Code_tUwOSwbyRm

Adding a proper test is blocked on #9134.

kLabz added a commit that referenced this issue Nov 29, 2023
@kLabz
Copy link
Contributor

kLabz commented Nov 29, 2023

@Simn do you want the test to do something more here?

@Simn
Copy link
Member

Simn commented Nov 29, 2023

The replace range should always be tested for these.

kLabz added a commit that referenced this issue Nov 30, 2023
0b1kn00b pushed a commit to 0b1kn00b/haxe that referenced this issue Jan 25, 2024
0b1kn00b pushed a commit to 0b1kn00b/haxe that referenced this issue Jan 25, 2024
0b1kn00b pushed a commit to 0b1kn00b/haxe that referenced this issue Jan 25, 2024
0b1kn00b pushed a commit to 0b1kn00b/haxe that referenced this issue Jan 25, 2024
kLabz pushed a commit that referenced this issue Jul 3, 2024
Note: not including texprConverter changes

see 160a490
see #7282
kLabz added a commit that referenced this issue Jul 3, 2024
kLabz added a commit that referenced this issue Jul 18, 2024
* [display] diagnostics as json rpc (Backport #11412)

* [tests] use json rpc diagnostics

* [tests] Add test for 11695

* [tests] Update diagnostics tests

* Run some filters in diagnostics (#11220)

* let's see how much breaks

* [tests] enable diagnostics tests for 11177 and 11184

* [tests] Update test for 5306

* Don't cache/run filters for find reference/implementation requests (#11226)

* Only run filters and save cache on diagnostics, not usage requests

* [tests] Update test for 11184

* disable test

* add VUsedByTyper to avoid bad unused local errors

* revert @:compilerGenerated change

---------

Co-authored-by: Rudy Ges <k@klabz.org>

* [display] get rid of TypeloadParse.current_stdin

* [display] fix -D display-stdin handling

* [display] generalize fileContents behavior to other json rpc display calls

* [display] fix range of pattern variables

Note: not including texprConverter changes

see 160a490
see #7282

* [tests] add test for #7282

* [tests] add test for #7931

* Remove populateCacheFromDisplay config

Legacy diagnostics = false, json rpc diagnostics = true

* [std] Diagnostics request doc

* [tests] Test Json RPC diagnostics with several open files

* [diagnostics] fix multi display files (#11722)

* [diagnostics] fix json rpc diagnostics display config

* [tests] Server tests: do not fail silently when runHaxeJsonCb errors

* [tests] add more diagnostics tests

* [display] rework multiple display files handling

* clean up a bit...

* [diagnostics] handle a.b.c.hx case, even if pointless

* [diagnostics] do not skip errors during DisplayProcessing.process_display_file

* Enable display tests again...

* [tests] fix display tests

---------

Co-authored-by: Simon Krajewski <simon@haxe.org>
@kLabz kLabz mentioned this issue Jul 18, 2024
kLabz added a commit that referenced this issue Jul 18, 2024
* [display] diagnostics as json rpc (Backport #11412)

* [tests] use json rpc diagnostics

* [tests] Add test for 11695

* [tests] Update diagnostics tests

* Run some filters in diagnostics (#11220)

* let's see how much breaks

* [tests] enable diagnostics tests for 11177 and 11184

* [tests] Update test for 5306

* Don't cache/run filters for find reference/implementation requests (#11226)

* Only run filters and save cache on diagnostics, not usage requests

* [tests] Update test for 11184

* disable test

* add VUsedByTyper to avoid bad unused local errors

* revert @:compilerGenerated change

---------

Co-authored-by: Rudy Ges <k@klabz.org>

* [display] get rid of TypeloadParse.current_stdin

* [display] fix -D display-stdin handling

* [display] generalize fileContents behavior to other json rpc display calls

* [display] fix range of pattern variables

Note: not including texprConverter changes

see 160a490
see #7282

* [tests] add test for #7282

* [tests] add test for #7931

* Remove populateCacheFromDisplay config

Legacy diagnostics = false, json rpc diagnostics = true

* [std] Diagnostics request doc

* [tests] Test Json RPC diagnostics with several open files

* [diagnostics] fix multi display files (#11722)

* [diagnostics] fix json rpc diagnostics display config

* [tests] Server tests: do not fail silently when runHaxeJsonCb errors

* [tests] add more diagnostics tests

* [display] rework multiple display files handling

* clean up a bit...

* [diagnostics] handle a.b.c.hx case, even if pointless

* [diagnostics] do not skip errors during DisplayProcessing.process_display_file

* Enable display tests again...

* [tests] fix display tests

---------

Co-authored-by: Simon Krajewski <simon@haxe.org>
@kLabz
Copy link
Contributor

kLabz commented Jul 18, 2024

Should this be closed now or is there something still missing ?

@Simn Simn closed this as completed Jul 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature-ide IDE / Editor support
Projects
None yet
Development

No branches or pull requests

4 participants