Warn if Debug.Log is used in code

[borisbauer-unity]

A new code analysis for symbol "UnityEngine.Debug.Log".

Detail: I chose id PAC0192 since it seems the good spot for various issues before network issues starting at PAC0200.

[borisbauer-unity]

I think we need to warn about LogFormat too and maybe LogWarning, LogWarningFormat? In addition, it would be great to check if the caller method is tagged with the Conditionalattribute. If we wanted to do that, we would need to use a ICodeModuleInstructionAnalyzer. What do you think?

Makes sense, the conditional attribute on the caller method is interesting since we recommend it, and to learn about that part of the code.

Strictly speaking, the method the Log/Format/Warning is inside could have the conditional attribute and any caller in the callstack, right?

What is probably near impossible: Telling if the condition is actually stripping this out in the release, e.g. in case the users use some custom symbol.

[mtrive]

Strictly speaking, the method the Log/Format/Warning is inside could have the conditional attribute and any caller in the callstack, right?

Exactly. It should be possible. I can help if needed.

What is probably near impossible: Telling if the condition is actually stripping this out in the release, e.g. in case the users use some custom symbol.

That's correct. We cannot determine if the call to the Debug Log wrapper is stripped out, if that's what you mean.

[borisbauer-unity]

I can help if needed.

I'll look a bit at the other analyzers and see if I see any clues how to get to the caller and method attributes.

[borisbauer-unity]

Getting close: I found the right code for the ConditionalAttribute and can match our Debug methods.

Q: Should it be possible to actually crawl the stack backwards from within the ICodeModuleInstructionAnalyzer's Analysis method? ...to see if any callers further up have the ConditionalAttribute.

[mtrive]

Getting close: I found the right code for the ConditionalAttribute and can match our Debug methods.

Q: Should it be possible to actually crawl the stack backwards from within the ICodeModuleInstructionAnalyzer's Analysis method? ...to see if any callers further up have the ConditionalAttribute.

It's not something we have done so far and is not currently possible. Perhaps something we can look at in the future.

[borisbauer-unity]

New unit tests and fixed CodeAnalysisTests run ok now.

[mtrive]

@unitystevem for awareness since we talked about this diagnostic on several occations.

[unitystevem]

static readonly Descriptor k_DebugWarningIssueDescriptor = new Descriptor ( "PAC0193", "Debug.Warning / Debug.WarningFormat", Area.CPU, "Debug.Warning methods cause slowdowns, especially if used frequently.", "Instead of removing code an option is to strip this code on release builds by using scripting symbols for conditional compilation (#if ... #endif) or the ConditionalAttribute on a method where you call this. When logging is still used in your code a small optimization can be to leave out the callstack, if not required, by setting 'Application.SetStackTraceLogType(LogType.Warning, StackTraceLogType.None)' via code." )

There is no Unity API called Debug.Warning or Debug.WarningFormat. The methods are called Debug.LogWarning and Debug.LogWarningFormat.

[EDIT: With apologies for the mangled code formatting, I don't know how to fix it in this comment)