OSOE-819: Constant from JSON source generator

[AydinE]

OSOE-819
Also see: Lombiq/Helpful-Libraries#251
Lombiq/Orchard-Vue.js#134
Lombiq/Orchard-Base-Theme#109

[sarahelsaig]

For some reason I get these with your latest commit:

But if I check out the commit before that (92bbfa5) and do a clean and rebuild then it works. However by "works" I mean that I get no errors or warnings in VS from "Build Only". I still see IntelliSense errors:

[github-actions]

This pull request has merge conflicts. Please resolve those before requesting a review.

[AydinE]

@sarahelsaig please try again with the latest version, as for the remaining warnings in visual studio I can't seem to fully get rid of them but they don't seem to be an issue as the build does actually complete succesfully and the constant values are actually showing up in the editor also.

[sarahelsaig]

It works in VS without any build errors or warnings (at least on a freshly cloned repository), so that's good. But I get this warning in Intellisense-only mode:

The dll file exists in that location, so this makes no sense. I would say we can ignore it, but I would also say "throw VS in a volcano" so I'm biased here. @Piedone , what's your opinion on this warning?

[Piedone]

I really don't want us to have a warning that's actually OK. It's detrimental conditioning for the development team.

It doesn't matter if it's generated somewhere, but it shouldn't be surfaced as a warning. If you can turn it into a message, or otherwise hide it, then that's fine.

[AydinE]

@sarahelsaig @Piedone
After trying this out many more times today I found the following:
Warning shows up when you do a clean pull and build solution for the first time. If you close Visual Studio right after that and restart it the warning never seems to show up again even after cleaning and rebuilding the solution.

So it does really look like a weird issue with VS in this instance. We could add a <NoWarn> for this specific warning code to each project that uses the source generator but not sure that it's really the solution and seems kind of ugly.

Let me know what you both think we should do in this case

[Piedone]

Related, perhaps? dotnet/roslyn#54899 Though it's about native assemblies. Isn't the issue related to the assembly targeting netstandard? It can target .NET 8 too, it'll be used by OC modules, which are all .NET 8.

[AydinE]

Related, perhaps? dotnet/roslyn#54899 Though it's about native assemblies. Isn't the issue related to the assembly targeting netstandard? It can target .NET 8 too, it'll be used by OC modules, which are all .NET 8.

I did come across that post and it does seem to be about referencing native assemblies from the source generator. In our case it's about a local reference from a project referencing / using the source generator.

As for targetting netstandard2.0 from the source generator this is unfortunately a hard requirement for source generators to work.

[Piedone]

Then please suppress this warning if it's possible to do just the affected projects, commented why it's necessary, pointing to some common docs in HL where you also tell consumers to do this in their projects.

[AydinE]

@sarahelsaig @Piedone
Even when adding the NoWarn to try and suppress this warning I am still getting it in VS the first time I clone this project. Can either of you verify that it's still the case?

If so I am kind of running out of ideas on how to suppress this warning any further or what else to try, and it's starting to take up way too much time digging into this niche issue. Open to any suggestions though so please let me know how you think we should proceed.

[Piedone]

For me too. However, the DLL actually indeed doesn't exist for me when opening the solution. So, perhaps you need to fix that instead?

[AydinE]

For me too. However, the DLL actually indeed doesn't exist for me when opening the solution. So, perhaps you need to fix that instead?

Indeed it doesn't exist when the project is freshly cloned and opened for the first time in VS. It does however exist after the first build, I guess that is just a quirk of referencing source generators locally in this way. After that first build the warning doesn't seem to come back anymore (even after a clean of the solution).

Again I am open for suggestions on how to either solve this or suppress the warning further but as I mentioned I have kind of run out of things to try at this point.

Only thing I can think of right now is maybe open up an issue over at https://github.com/dotnet/roslyn and see if they have any ideas?

[Piedone]

Well, I suggest fixing that the DLL doesn't exist :). The warning is not a false positive. You can either make the DLL exist (perhaps by hooking into an MSBuild target that's executed when the project is loaded and doing some kind of pre-build) or the reference to only be active when it exists (you can add conditions to csproj elements).

Feel free to additionally ask under the Roslyn repo too (though I suggest a discussion, since this is a question, not a bug of Roslyn).

[github-actions]

This pull request has merge conflicts. Please resolve those before requesting a review.

[AydinE]

Well, I suggest fixing that the DLL doesn't exist :). The warning is not a false positive. You can either make the DLL exist (perhaps by hooking into an MSBuild target that's executed when the project is loaded and doing some kind of pre-build) or the reference to only be active when it exists (you can add conditions to csproj elements).

Feel free to additionally ask under the Roslyn repo too (though I suggest a discussion, since this is a question, not a bug of Roslyn).

@Piedone See the reply on the discusson dotnet/roslyn#72937 (comment) please let me know how you want to proceed.

[AydinE]

Hey @sarahelsaig ,

I think we are finally ready to review this again and hopefully actually merge it in this time!

[github-actions]

This pull request has merge conflicts. Please resolve those before requesting a review.

[github-actions]

This pull request has merge conflicts. Please resolve those before requesting a review.

[github-actions]

This pull request has merge conflicts. Please resolve those before requesting a review.

[AydinE]

Hey @sarahelsaig with those issues resolved is it ready to be merged?