From 760ec98ce1ae26df09de438429b6e60e866ed2dc Mon Sep 17 00:00:00 2001 From: comintern Date: Fri, 3 Jun 2016 21:26:02 -0500 Subject: [PATCH] Fix ByRef, ParamArray, and array parameter declarations. (#1692) * Fix ByRef and array determinations, added ParamArray support. * Forgot to remove some comments from test. --- Rubberduck.Parsing/Rubberduck.Parsing.csproj | 1 + Rubberduck.Parsing/Symbols/ComParameter.cs | 21 +++++ .../Symbols/ParameterDeclaration.cs | 7 +- .../ReferencedDeclarationsCollector.cs | 77 ++++++++++--------- .../Settings/IndenterSettingsTests.cs | 40 ---------- 5 files changed, 64 insertions(+), 82 deletions(-) create mode 100644 Rubberduck.Parsing/Symbols/ComParameter.cs diff --git a/Rubberduck.Parsing/Rubberduck.Parsing.csproj b/Rubberduck.Parsing/Rubberduck.Parsing.csproj index 3e464bab57..f8c4adea1c 100644 --- a/Rubberduck.Parsing/Rubberduck.Parsing.csproj +++ b/Rubberduck.Parsing/Rubberduck.Parsing.csproj @@ -128,6 +128,7 @@ + diff --git a/Rubberduck.Parsing/Symbols/ComParameter.cs b/Rubberduck.Parsing/Symbols/ComParameter.cs new file mode 100644 index 0000000000..f94507073f --- /dev/null +++ b/Rubberduck.Parsing/Symbols/ComParameter.cs @@ -0,0 +1,21 @@ +using System; +using System.Collections.Generic; +using System.Linq; +using System.Text; +using System.Threading.Tasks; + +namespace Rubberduck.Parsing.Symbols +{ + public class ComParameter + { + public bool IsArray { get; set; } + public bool IsByRef { get; set;} + public string Name { get; set;} + + public ComParameter(string name, bool byRef) + { + Name = name; + IsByRef = byRef; + } + } +} diff --git a/Rubberduck.Parsing/Symbols/ParameterDeclaration.cs b/Rubberduck.Parsing/Symbols/ParameterDeclaration.cs index 14f628350e..bf030dc300 100644 --- a/Rubberduck.Parsing/Symbols/ParameterDeclaration.cs +++ b/Rubberduck.Parsing/Symbols/ParameterDeclaration.cs @@ -8,7 +8,6 @@ public class ParameterDeclaration : Declaration { private readonly bool _isOptional; private readonly bool _isByRef; - private readonly bool _isParamArray; /// /// Creates a new built-in parameter declaration. @@ -39,7 +38,7 @@ public ParameterDeclaration(QualifiedMemberName qualifiedName, { _isOptional = isOptional; _isByRef = isByRef; - _isParamArray = isParamArray; + IsParamArray = isParamArray; } /// @@ -74,11 +73,11 @@ public ParameterDeclaration(QualifiedMemberName qualifiedName, { _isOptional = isOptional; _isByRef = isByRef; - _isParamArray = isParamArray; + IsParamArray = isParamArray; } public bool IsOptional { get { return _isOptional; } } public bool IsByRef { get { return _isByRef; } } - public bool IsParamArray { get { return _isParamArray; } } + public bool IsParamArray { get; set; } } } diff --git a/Rubberduck.Parsing/Symbols/ReferencedDeclarationsCollector.cs b/Rubberduck.Parsing/Symbols/ReferencedDeclarationsCollector.cs index 9d8aa7cb16..12821de6fe 100644 --- a/Rubberduck.Parsing/Symbols/ReferencedDeclarationsCollector.cs +++ b/Rubberduck.Parsing/Symbols/ReferencedDeclarationsCollector.cs @@ -55,6 +55,8 @@ public ReferencedDeclarationsCollector(RubberduckParserState state) _state = state; } + private static readonly HashSet IgnoredInterfaceMembers = new HashSet { "QueryInterface", "AddRef", "Release", "GetTypeInfoCount", "GetTypeInfo", "GetIDsOfNames", "Invoke" }; + private static readonly IDictionary TypeNames = new Dictionary { {VarEnum.VT_DISPATCH, "DISPATCH"}, @@ -87,7 +89,7 @@ public ReferencedDeclarationsCollector(RubberduckParserState state) private readonly Dictionary _comInformation = new Dictionary(); - private string GetTypeName(TYPEDESC desc, ITypeInfo info) + private ComParameter GetParameterInfo(TYPEDESC desc, ITypeInfo info) { var vt = (VarEnum)desc.vt; TYPEDESC tdesc; @@ -96,7 +98,9 @@ private string GetTypeName(TYPEDESC desc, ITypeInfo info) { case VarEnum.VT_PTR: tdesc = (TYPEDESC)Marshal.PtrToStructure(desc.lpValue, typeof(TYPEDESC)); - return GetTypeName(tdesc, info); + var pointer = GetParameterInfo(tdesc, info); + pointer.IsByRef = true; + return pointer; case VarEnum.VT_USERDEFINED: int href; unchecked @@ -107,24 +111,29 @@ private string GetTypeName(TYPEDESC desc, ITypeInfo info) { ITypeInfo refTypeInfo; info.GetRefTypeInfo(href, out refTypeInfo); - return GetTypeName(refTypeInfo); + return new ComParameter(GetTypeName(refTypeInfo), false); } catch (Exception) { - return "Object"; + return new ComParameter("Object", false); } + case VarEnum.VT_SAFEARRAY: case VarEnum.VT_CARRAY: + case VarEnum.VT_ARRAY: tdesc = (TYPEDESC)Marshal.PtrToStructure(desc.lpValue, typeof(TYPEDESC)); - return GetTypeName(tdesc, info) + "()"; + var array = GetParameterInfo(tdesc, info); + array.IsArray = true; + array.Name += "()"; + return array; default: string result; if (TypeNames.TryGetValue(vt, out result)) { - return result; + return new ComParameter(result, false); } break; } - return "Object"; + return new ComParameter("Object", false); } private string GetTypeName(ITypeInfo info) @@ -191,10 +200,9 @@ public List GetDeclarationsForReference(Reference reference) IntPtr typeAttributesPointer; info.GetTypeAttr(out typeAttributesPointer); - var typeAttributes = (TYPEATTR)Marshal.PtrToStructure(typeAttributesPointer, typeof(TYPEATTR)); - var attributes = new Attributes(); + if (typeAttributes.wTypeFlags.HasFlag(TYPEFLAGS.TYPEFLAG_FPREDECLID)) { attributes.AddPredeclaredIdTypeAttribute(); @@ -294,8 +302,8 @@ memberDeclaration is ICanBeDefaultMember && } output.Add(memberDeclaration); - var parameterCount = memberDescriptor.cParams - - (memberDescriptor.invkind.HasFlag(INVOKEKIND.INVOKE_PROPERTYGET) ? 0 : 1); + var parameterCount = memberDescriptor.cParams - (memberDescriptor.invkind.HasFlag(INVOKEKIND.INVOKE_PROPERTYGET) ? 0 : 1); + var parameters = new List(); for (var paramIndex = 0; paramIndex < parameterCount; paramIndex++) { var parameter = CreateParameterDeclaration(memberNames, paramIndex, memberDescriptor, @@ -303,11 +311,16 @@ memberDeclaration is ICanBeDefaultMember && var declaration = memberDeclaration as IDeclarationWithParameter; if (declaration != null) { + parameters.Add(parameter); declaration.AddParameter(parameter); } output.Add(parameter); } member.TypeInfo.ReleaseFuncDesc(memberDescriptorPointer); + if (parameters.Any() && memberDescriptor.cParamsOpt == -1) + { + parameters.Last().IsParamArray = true; + } } for (var fieldIndex = 0; fieldIndex < member.TypeAttributes.cVars; fieldIndex++) @@ -337,10 +350,10 @@ private Declaration CreateMemberDeclaration(FUNCDESC memberDescriptor, TYPEKIND var funcValueType = (VarEnum)memberDescriptor.elemdescFunc.tdesc.vt; var memberDeclarationType = GetDeclarationType(memberName, memberDescriptor, funcValueType, typeKind, parentImplFlags); - var asTypeName = string.Empty; + var asTypeName = new ComParameter(string.Empty, false); if (memberDeclarationType != DeclarationType.Procedure) { - asTypeName = GetTypeName(memberDescriptor.elemdescFunc.tdesc, info); + asTypeName = GetParameterInfo(memberDescriptor.elemdescFunc.tdesc, info); } var attributes = new Attributes(); if (memberName == "_NewEnum" && ((FUNCFLAGS)memberDescriptor.wFuncFlags).HasFlag(FUNCFLAGS.FUNCFLAG_FNONBROWSABLE)) @@ -363,7 +376,7 @@ private Declaration CreateMemberDeclaration(FUNCDESC memberDescriptor, TYPEKIND new QualifiedMemberName(typeQualifiedModuleName, memberName), moduleDeclaration, moduleDeclaration, - asTypeName, + asTypeName.Name, Accessibility.Global, null, Selection.Home, @@ -375,14 +388,13 @@ private Declaration CreateMemberDeclaration(FUNCDESC memberDescriptor, TYPEKIND new QualifiedMemberName(typeQualifiedModuleName, memberName), moduleDeclaration, moduleDeclaration, - asTypeName, + asTypeName.Name, null, null, Accessibility.Global, null, Selection.Home, - // TODO: how to find out if it's an array? - false, + asTypeName.IsArray, true, null, attributes); @@ -391,14 +403,13 @@ private Declaration CreateMemberDeclaration(FUNCDESC memberDescriptor, TYPEKIND new QualifiedMemberName(typeQualifiedModuleName, memberName), moduleDeclaration, moduleDeclaration, - asTypeName, + asTypeName.Name, null, null, Accessibility.Global, null, Selection.Home, - // TODO: how to find out if it's an array? - false, + asTypeName.IsArray, true, null, attributes); @@ -407,7 +418,7 @@ private Declaration CreateMemberDeclaration(FUNCDESC memberDescriptor, TYPEKIND new QualifiedMemberName(typeQualifiedModuleName, memberName), moduleDeclaration, moduleDeclaration, - asTypeName, + asTypeName.Name, Accessibility.Global, null, Selection.Home, @@ -419,7 +430,7 @@ private Declaration CreateMemberDeclaration(FUNCDESC memberDescriptor, TYPEKIND new QualifiedMemberName(typeQualifiedModuleName, memberName), moduleDeclaration, moduleDeclaration, - asTypeName, + asTypeName.Name, Accessibility.Global, null, Selection.Home, @@ -431,7 +442,7 @@ private Declaration CreateMemberDeclaration(FUNCDESC memberDescriptor, TYPEKIND new QualifiedMemberName(typeQualifiedModuleName, memberName), moduleDeclaration, moduleDeclaration, - asTypeName, + asTypeName.Name, null, false, false, @@ -462,11 +473,11 @@ private Declaration CreateFieldDeclaration(ITypeInfo info, int fieldIndex, Decla var fieldName = names[0]; var memberType = GetDeclarationType(varDesc, typeDeclarationType); - var asTypeName = GetTypeName(varDesc.elemdescVar.tdesc, info); + var asTypeName = GetParameterInfo(varDesc.elemdescVar.tdesc, info); info.ReleaseVarDesc(ppVarDesc); return new Declaration(new QualifiedMemberName(typeQualifiedModuleName, fieldName), - moduleDeclaration, moduleDeclaration, asTypeName, null, false, false, Accessibility.Global, memberType, null, + moduleDeclaration, moduleDeclaration, asTypeName.Name, null, false, false, Accessibility.Global, memberType, null, Selection.Home, false, null); } @@ -478,20 +489,10 @@ private ParameterDeclaration CreateParameterDeclaration(IReadOnlyList me var paramPointer = new IntPtr(memberDescriptor.lprgelemdescParam.ToInt64() + Marshal.SizeOf(typeof(ELEMDESC)) * paramIndex); var elementDesc = (ELEMDESC)Marshal.PtrToStructure(paramPointer, typeof(ELEMDESC)); var isOptional = elementDesc.desc.paramdesc.wParamFlags.HasFlag(PARAMFLAG.PARAMFLAG_FOPT); - - var isByRef = elementDesc.desc.paramdesc.wParamFlags.HasFlag(PARAMFLAG.PARAMFLAG_FOUT); - var isArray = false; var paramDesc = elementDesc.tdesc; - var valueType = (VarEnum)paramDesc.vt; - if (valueType == VarEnum.VT_CARRAY || valueType == VarEnum.VT_ARRAY || valueType == VarEnum.VT_SAFEARRAY) - { - // todo: tell ParamArray arrays from normal arrays - isArray = true; - } - - var asParamTypeName = GetTypeName(paramDesc, info); + var paramInfo = GetParameterInfo(paramDesc, info); - return new ParameterDeclaration(new QualifiedMemberName(typeQualifiedModuleName, paramName), memberDeclaration, asParamTypeName, null, null, isOptional, isByRef, isArray); + return new ParameterDeclaration(new QualifiedMemberName(typeQualifiedModuleName, paramName), memberDeclaration, paramInfo.Name, null, null, isOptional, paramInfo.IsByRef, paramInfo.IsArray); } private IEnumerable GetImplementedInterfaceNames(TYPEATTR typeAttr, ITypeInfo info) @@ -587,7 +588,7 @@ private DeclarationType GetDeclarationType(string memberName, FUNCDESC funcDesc, } else if ((parentImplTypeFlags.HasFlag(IMPLTYPEFLAGS.IMPLTYPEFLAG_FSOURCE) || ((FUNCFLAGS)funcDesc.wFuncFlags).HasFlag(FUNCFLAGS.FUNCFLAG_FSOURCE)) && - !new[] {"QueryInterface", "AddRef", "Release", "GetTypeInfoCount", "GetTypeInfo", "GetIDsOfNames", "Invoke"}.Contains(memberName)) // quick-and-dirty for beta + !IgnoredInterfaceMembers.Contains(memberName)) // quick-and-dirty for beta { memberType = DeclarationType.Event; } diff --git a/RubberduckTests/Settings/IndenterSettingsTests.cs b/RubberduckTests/Settings/IndenterSettingsTests.cs index 56216ea7d2..2057f1081b 100644 --- a/RubberduckTests/Settings/IndenterSettingsTests.cs +++ b/RubberduckTests/Settings/IndenterSettingsTests.cs @@ -91,52 +91,12 @@ public static Rubberduck.SmartIndenter.IndenterSettings GetMockIndenterSettings( private Configuration GetDefaultConfig() { - //var indenterSettings = GetTestIndenterSettings(); - //{ - // indenterSettings.IndentEntireProcedureBody = true, - // indenterSettings.IndentFirstCommentBlock = true, - // indenterSettings.IndentFirstDeclarationBlock = true, - // indenterSettings.AlignCommentsWithCode = true, - // indenterSettings.AlignContinuations = true, - // IgnoreOperatorsInContinuations = true, - // IndentCase = false, - // ForceDebugStatementsInColumn1 = false, - // ForceCompilerDirectivesInColumn1 = false, - // IndentCompilerDirectives = true, - // AlignDims = false, - // AlignDimColumn = 15, - // EnableUndo = true, - // EndOfLineCommentStyle = EndOfLineCommentStyle.AlignInColumn, - // EndOfLineCommentColumnSpaceAlignment = 50, - // IndentSpaces = 4 - //}; - var userSettings = new UserSettings(null, null, null, null, null, GetMockIndenterSettings()); return new Configuration(userSettings); } private Configuration GetNondefaultConfig() { - //var indenterSettings = new Rubberduck.SmartIndenter.IndenterSettings - //{ - // IndentEntireProcedureBody = false, - // IndentFirstCommentBlock = false, - // IndentFirstDeclarationBlock = false, - // AlignCommentsWithCode = false, - // AlignContinuations = false, - // IgnoreOperatorsInContinuations = false, - // IndentCase = true, - // ForceDebugStatementsInColumn1 = true, - // ForceCompilerDirectivesInColumn1 = true, - // IndentCompilerDirectives = false, - // AlignDims = true, - // AlignDimColumn = 16, - // EnableUndo = false, - // EndOfLineCommentStyle = EndOfLineCommentStyle.Absolute, - // EndOfLineCommentColumnSpaceAlignment = 60, - // IndentSpaces = 2 - //}; - var userSettings = new UserSettings(null, null, null, null, null, GetMockIndenterSettings(true)); return new Configuration(userSettings); }