Skip to content

OverrideTypeReaderAttribute incorrectly applies to other parameters of the same type #936

@glen3b

Description

@glen3b
[glen@UltimateBuilder ~]$ dotnet --version
2.1.3
[glen@UltimateBuilder ~]$ dotnet --info
.NET Command Line Tools (2.1.3)

Product Information:
 Version:            2.1.3
 Commit SHA-1 hash:  a0ca411ca5

Runtime Environment:
 OS Name:     arch
 OS Version:  
 OS Platform: Linux
 RID:         linux-x64
 Base Path:   /opt/dotnet/sdk/2.1.3/

Microsoft .NET Core Shared Framework Host

  Version  : 2.0.4
  Build    : 7f262f453d8c8479b9af91d34c013b3aa05bc1ff

[glen@UltimateBuilder ~]$ 

Tested with Discord.Net AppVeyor build version 2.0.0-beta-00880 (for commit 05cd1ff).


Consider the following two classes, a type reader and a module (the type reader is not registered with the command service for any type - see the main method):

// For demonstration purposes; a real "validating" type reader would be more useful
public class NumericStringTypeReader : TypeReader
{
    public override Task<TypeReaderResult> ReadAsync(ICommandContext context, string input, IServiceProvider services)
    {
        if (int.TryParse(input, out int val))
        {
            return Task.FromResult(TypeReaderResult.FromSuccess(input));
        }
        return Task.FromResult(TypeReaderResult.FromError(CommandError.ParseFailed, "Could not parse stringy-integer argument."));
    }
}

public class TestCommandModule : ModuleBase
{
    [Command("stringintecho")]
    public Task StringIntEcho([OverrideTypeReader(typeof(NumericStringTypeReader))] string argument) => ReplyAsync("Message (stringIntEcho): " + argument);
    [Command("stringecho")]
    public Task StringEcho(string argument) => ReplyAsync("Message (stringEcho): " + argument);
}

Expected behavior:

  • The stringecho command echoes any argument given without error
  • Due to the presence of the overriding type reader, the stringintecho method only accepts and echoes parseable integers

Reality:

  • Both commands are restricted via the type reader (type reader's invocation evidenced by the distinct parse error message) to accepting only parseable integers
    (Note especially the first command invocation in the screenshot)
    message screenshot

Notes:
This issue does not occur if the declaration order of the commands is reversed. However, the order of invocation once the bot has started appears to be irrelevant.

The dependence on the order of declaration leads me to believe that the issue is in ModuleClassBuilder - BuildParameter's logic for the default type reader of a parameter, called while the module is built:

            if (builder.TypeReader == null)
            {
                var readers = service.GetTypeReaders(paramType);
                TypeReader reader = null;

                if (readers != null)
                    reader = readers.FirstOrDefault().Value;
                else
                    reader = service.GetDefaultTypeReader(paramType);

                builder.TypeReader = reader;
            }

GetTypeReaders returns an IDictionary<Type, TypeReader>, which appears to be mapping from Types of TypeReaders to their cached instances. This dictionary is populated in the GetTypeReader method, called earlier in our example by the BuildParameters invocation for our "stringintecho" command. "stringecho"'s parameter is of type string with no explicit type reader. Since "stringintecho" has already been built, however, readers is non-null and non-empty, containing a NumericStringTypeReader. However, the default type reader is desired. Since GetDefaultTypeReader already uses an internal dictionary for caching instances of the default (correct) type reader type, unless I'm missing something, this whole default case can be simplified to:

            if (builder.TypeReader == null)
            {
                builder.TypeReader = service.GetDefaultTypeReader(paramType);
            }

because if a type reader has not been specified, we don't want a random initialized type reader from the cache; we want the default for that type.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions