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

Run some filters in diagnostics #11220

Merged
merged 12 commits into from
Nov 21, 2023
Merged

Conversation

Simn
Copy link
Member

@Simn Simn commented May 13, 2023

Opening PR so I don't forget, because I kind of did.

Something is still failing which has to be investigated.

@kLabz
Copy link
Contributor

kLabz commented May 16, 2023

Problem is, this (and 7321f18) also run on DMUsage and DMImplementation requests which seem different enough (dms_inline being false for example) to break (during filters atm and possibly at other points?)

As said on slack, there are failures here for find reference requests (for example on 9044 and 9446 display tests), hitting a die + assertion failure while running RenameVars filter (determine_overlaps runs into unexpected state in haxe.macro.Printer.printField for example because of an inline local function).

Checking other failures (damn CI noise with network requests failing again...)


Macro failures seem to be mostly the same, with lots of Failed to locate variable declaration failures for @usage display requests, similarly to above (while handling haxe.format.JsonParser mostly?)

Seems to be hanging locally too if I don't avoid the die on these errors.

…11226)

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

* [tests] Update test for 11184
@RblSb
Copy link
Member

RblSb commented Jun 5, 2023

@kLabz
Copy link
Contributor

kLabz commented Jun 5, 2023

Let's see what CI has to say about it

@kLabz
Copy link
Contributor

kLabz commented Jun 5, 2023

Nope, still broken here.

@Simn
Copy link
Member Author

Simn commented Nov 21, 2023

#11229 wasn't a good fix for #11203. I'm not even sure why it does anything at all because that @:compilerGenerated metadata doesn't make it to Diagnostics.check_other_things:

function() {
  	var future = _Main.Future_Impl_._new();
  	haxe.Log.trace("much side effect!", {fileName : "source/Main.hx", lineNumber : 13, className : "_Main.Future_Impl_", methodName : "eager"});
  }

There's a chance that this fix was only incidental, though I don't know how that could happen yet.

It also doesn't deal with the situation where future is reported as an unused variable. I'll disable this test and, if nothing else fails, merge this PR. We can then look into fixing the original issue properly.

@Simn Simn marked this pull request as ready for review November 21, 2023 11:26
@Simn
Copy link
Member Author

Simn commented Nov 21, 2023

The story here is that #11229 generated this @:compilerGenerated metadata, but also changed reduce_expr to remove it again. Thus when running the filters, the metadata is gone and the original problem triggers.

This took me forever to properly reproduce because I didn't realize that our display tests run without a specified target. This means we have com.platform = Cross, which causes the analyzer to not run. The analyzer cleans up the useless block-level cast expression, so the issue doesn't reproduce on "real" diagnostics runs and is exclusive to the test.

@Simn Simn merged commit 486dfc8 into development Nov 21, 2023
121 of 122 checks passed
@Simn Simn deleted the run_some_filters_in_diagnostics branch November 21, 2023 14:56
0b1kn00b pushed a commit to 0b1kn00b/haxe that referenced this pull request Jan 25, 2024
* 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 (HaxeFoundation#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>
@kLabz kLabz mentioned this pull request Jul 3, 2024
19 tasks
kLabz added a commit that referenced this pull request Jul 3, 2024
* 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>
kLabz added a commit that referenced this pull request 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 added a commit that referenced this pull request 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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants