EngineHub / Intake

IoC-oriented Java command parsing library
GNU Lesser General Public License v3.0
98 stars 21 forks source link

BindingBehavior Validation is ambiguous #1

Closed Kiskae closed 9 years ago

Kiskae commented 10 years ago

The BindingBehavior used in the definition of custom parameters exhibits some weird validation that I do not believe is correct.

The following table shows the BindingBehavior, the given number of consumed arguments and whether the validator (ParameterData.java#L160) will allow it.

BindingBehavior -1 0 > 0
CONSUMES Allowed Error Allowed
INDETERMINATE Allowed Error Error
PROVIDES Allowed Error Error

This might seem correct, but the check at ParameterData.java#L186 leads me to assume that PROVIDES is allowed to have a consuming count of 0.

A clarification would be appreciated and if my assumption is true, the validation will need to be changed.

sk89q commented 10 years ago

PROVIDES means that no parameters will be consumed because they will come from somewhere else (CommandsLocals, etc.), so it can only be compatible with consumedCount=0, although it doesn't look like it enforces this.

INDETERMINATE should only allow -1 because the method doesn't know how many arguments it will consume. If it knew, it wouldn't be INDETERMINATE.

CONSUMES can't be 0 (because it's definitely consuming) but it can't be -1 because CONSUMES means that it is known.

Arguably, the enum is redundant because consumedCount exists, and for all intents and purposes, the count provides a superset of all the information that the enum provides. I should probably remove the enum. The enum might be an artifact from something else before the count came along.

Kiskae commented 10 years ago

PROVIDES means that no parameters will be consumed because they will come from somewhere else (CommandsLocals, etc.), so it can only be compatible with consumedCount=0, although it doesn't look like it enforces this.

It is not enforced and also doesn't pass validation, since it doesn't get past this check ParameterData.java#L176.

It does seem like the enum is redundant, removing it would also mean the validation can be removed since it only checks conflicts between the enum and the defined number of consumed arguments

sk89q commented 9 years ago

Fixed since I threw all of this away for 4.x. Itmight need to make a return in the future if I add back support for optional parameters that don't occur at the end.