String optional arguments & SA0001

Sep 29, 2010 at 2:23 PM
Edited Sep 29, 2010 at 2:26 PM

Hi guys!

When I tried to analyze this simple class:

public class A

{

    public void Func(string p = "")

}

I got a message:

SA0001: An exception occurred while parsing the file: System.ArgumentNullException, element
Parameter name: Must not be null
   at Microsoft.StyleCop.Param.RequireNotNull(Object parameter, String parameterName, ParamErrorTextHandler errorTextHandler)
   at Microsoft.StyleCop.Param.RequireNotNull(Object parameter, String parameterName)
   at Microsoft.StyleCop.StyleCopAddIn.AddViolation(ICodeElement element, Int32 line, String ruleName, Object[] values)
   at Microsoft.StyleCop.StyleCopAddIn.AddViolation(ICodeElement element, Int32 line, Enum ruleName, Object[] values)
   at Microsoft.StyleCop.CSharp.ReadabilityRules.CheckEmptyString(Node`1 stringNode)
   at Microsoft.StyleCop.CSharp.ReadabilityRules.IterateTokenList(CsDocument document, Settings settings)
   at Microsoft.StyleCop.CSharp.ReadabilityRules.AnalyzeDocument(CodeDocument document)
   at Microsoft.StyleCop.StyleCopThread.RunAnalyzers(CodeDocument document, SourceParser parser, IEnumerable`1 analyzers)
   at Microsoft.StyleCop.StyleCopThread.TestAndRunAnalyzers(CodeDocument document, SourceParser parser, IEnumerable`1 analyzers, Int32 passNumber)
   at Microsoft.StyleCop.StyleCopThread.ParseAndAnalyzeDocument(SourceCode sourceCode, DocumentAnalysisStatus documentStatus)
   at Microsoft.StyleCop.StyleCopThread.DoWork(Object sender, DoWorkEventArgs e). D:\TFS\Engineering\KL.StyleCop\Main\Source\UnitTests\Source\Samples\OptionalArguments.cs 1 1 

I am using StyleCop 4.4.0.14. Is it a known bug or not?

Thank you

 

Developer
Oct 1, 2010 at 7:18 PM

I added { } to the function to make it compile, and then reproduced this error.  It doesn't appear to be logged in the Issue Tracker yet.

Developer
Oct 1, 2010 at 8:14 PM
Edited Oct 1, 2010 at 8:17 PM

I recommend adding an issue to Issue Tracker for this.  I believe default parameters are new to C# 4.0 so this issue was probably just missed with the upgrade to support it.

I took a quick stab at debugging this issue, but have run out of time, and won't be able to continue (at least not today and may be a few days before I could reasonably attack it again)...  if someone else wants to pick up from here, please do.

In CheckEmptyString in ReadabilityRules.cs, @string.FindParentElement() is returning null.  (As a side note, "@string" really?  I think I'll add a request later for a rule to not use such characters in variable names, as there is always something better you can name it.  Off the top of my head, "tokenString" here works.  "@string.Something()" can mislead a coder to think "Is there some extension method of the "string" class AND some usage of the @ operator, that I'm unaware of?")

Anyway, my gut feeling, looking at the following statement, is that we need to add code to handle the default parameters cases:

if (previousToken == null || (previousToken.Value.CsTokenType != CsTokenType.Case && !IsConstVariableDeclaration(previousToken)))

Can be logic something like:

if (previousToken == null || (previousToken.Value.CsTokenType != CsTokenType.Case && !(IsConstVariableDeclaration(previousToken) || IsDefaultParameterization(previousToken))))

(At this point, it would make sense to add statements to simplify the if statement itself.)  Add an IsDefaultParameterization(Node<CsToken> assignmentOperator) method to follow the IsConstVariableDeclaration method which operates as you'd expect and I think that's all there is to it.

Oct 5, 2010 at 1:26 PM

Thank you karak! Of course I forgot a body for Func(...)

I have created issue  6755