Excel-DNA / ExcelDna

Excel-DNA - Free and easy .NET for Excel. This repository contains the core Excel-DNA library.
https://excel-dna.net
zlib License
1.26k stars 270 forks source link

Expand CustomAttributes for paramarray parameters #686

Open govert opened 3 months ago

govert commented 3 months ago

This from a user - when we expand a paramarray parameter, the newly created parameters don't get the attributes we had on the original paramarry parameter. They're asking us to copy the custom attributes to all the new parameters. That seems reasonable and safe to me.

I’ve been working with the Excel DNA libraries for our internal projects. I ran into a small issue with the handling of vararg in the code base. In ParamsRegistration.cs, a vararg is unpacked up to a maximum 125 length for the function. When this is done, the ExcelParameterRegistration attribute is created and added for each new argument. Unfortunately, any other attributes are not transferred, so we lose our flags for handling of arguments in special ways.

The original code was something like:

reg.ParameterRegistrations.Add(
new ExcelParameterRegistration( /* create ExcelArgumentAttribute */));

for (int i = 0; i < restCount; ++i) {
reg.ParameterRegistration.Add( /* create ExcelArgumentAttribute */));
}

This, of course, drops our attributes which creates the problem. What I did, instead, in the code was create the parameter registration and then transfer the attributes that had been on the argument:

var newRegistration = new ExcelParameterRegistration(/* create ExcelArgumentAttribute */));
newRegistration.CustomAttributes.AddRange(lastParam.CustomAttributes);
reg.ParameterRegistration.Add(newRegistration);

The ParamArrayAttribute was already stripped from the last parameter (and others could be if a full transfer isn’t desired), but this passes along the developer’s attributes for later use when coercing the parameters.

@Sergey-Vlasov Does this look OK to you? I think 'sharing' the attributes objects between different parameters while using the MethodBuilder should be OK, but it's worth thinking or checking that nothing goes wrong.

Sergey-Vlasov commented 1 week ago

Implemented in https://github.com/Excel-DNA/ExcelDna/pull/698