UnityEngineAnalyzer icon indicating copy to clipboard operation
UnityEngineAnalyzer copied to clipboard

Add serveral new analysis

Open donaldwuid opened this issue 8 years ago • 11 comments

DoNotUseCameraMainInUpdate ShouldCacheDelegate Remove DoNotUseForeachInUpdate DoNotBoxWhenInvoke DoNotUseEnumTypeParameter StructShouldImplementIEquatable StructShouldOverride DoNotUseMaterialName

donaldwuid avatar Oct 08 '17 05:10 donaldwuid

Thanks for contributing @donaldwuid ! I will be merging some of your analyzers - some of the analyzers are dupes with other PRs. The I also noticed that you removed the DoNotUseForEachInUpdate which is no longer an issue with newer versions of unity - but it's still an issue with older versions of unity. I'd like to add in some sort of a version check to determine which analyzers to run on which versions - what are your thoughts on this?

vad710 avatar Oct 08 '17 13:10 vad710

@vad710 I think version check is necessary: https://github.com/vad710/UnityEngineAnalyzer/pull/12 is only relevant in >= Unity 5.3.

From CLI, we know the project path, so we could then try to read ./ProjectSettings/ProjectVersion.txt ? or let user define it as parameter?

Kasperki avatar Oct 08 '17 20:10 Kasperki

hello, I fix several bugs and do some improvement

donaldwuid avatar Oct 09 '17 13:10 donaldwuid

If we can know the CLI version precisely, we can use it. But if we can NOT, or it's too hard for us to make it, I suggest that our analyzer should be Conservative and just comment out the for-each analyzer.

donaldwuid avatar Oct 09 '17 13:10 donaldwuid

@donaldwuid I think that with the new config file that @Kasperki wrote, you can specify which analyzers you want to run - I think we should keep the ForEach analyzer for now and those that don't want to use it can omit it via the config (it can even by default).

In the meantime, could you please update the PR so that there's no conflicts and has no duplicate Analyzers?

vad710 avatar Oct 16 '17 23:10 vad710

The branch still has conflicts even after latest commit. I'll review the changes once the conflicts have been resolved.

vad710 avatar Oct 17 '17 18:10 vad710

Sorry about the delay. I'm using mac personally, and I can only access PC & VisualStudio during workdays. These conflicts will be done on tomorrow Monday.

By the way, since this PR, in my branch, I've commit several bug fixes, and add one Unity editor script to add our UnityEngineAnalysis.dll to csproj automatically.

donaldwuid avatar Oct 22 '17 01:10 donaldwuid

I've began porting over a lot of the code to dotnet standard and will eventually create a mono or dotnet core version of the CLI so that folks who use Mac/Linux can take advantage of the analyzers as well. You can see what we have here: https://github.com/vad710/UnityEngineAnalyzer/tree/dotnet-standard

vad710 avatar Oct 22 '17 10:10 vad710

please hold on, this is a merge without testing

donaldwuid avatar Oct 23 '17 07:10 donaldwuid

merged & test

donaldwuid avatar Oct 23 '17 11:10 donaldwuid

Hi @donaldwuid - it's not clear to me what the AutoAdd is for - are you enabling the Anaylzer in Visual Studio dynamically?

vad710 avatar Oct 23 '17 21:10 vad710