Skip to content

Commit d4cae87

Browse files
jonsequiturjonsequitur
authored
make ValidateSymbolResult void returning (#1567)
* make ValidateSymbolResult void returning, clean up file existence validators * update API approvals
1 parent ef7f2e8 commit d4cae87

File tree

11 files changed

+103
-118
lines changed

11 files changed

+103
-118
lines changed

‎src/System.CommandLine.ApiCompatibility.Tests/ApiCompatibilityApprovalTests.System_CommandLine_api_is_not_changed.approved.txt

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -535,5 +535,5 @@ System.CommandLine.Parsing
535535
public delegate ValidateSymbolResult<in T> : System.MulticastDelegate, System.ICloneable, System.Runtime.Serialization.ISerializable
536536
.ctor(System.Object object, System.IntPtr method)
537537
public System.IAsyncResult BeginInvoke(T symbolResult, System.AsyncCallback callback, System.Object object)
538-
public System.String EndInvoke(System.IAsyncResult result)
539-
public System.String Invoke(T symbolResult)
538+
public System.Void EndInvoke(System.IAsyncResult result)
539+
public System.Void Invoke(T symbolResult)

‎src/System.CommandLine.Tests/OptionTests.cs

Lines changed: 6 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -304,13 +304,12 @@ public void Option_T_default_value_factory_can_be_set_after_instantiation()
304304
public void Option_T_default_value_is_validated()
305305
{
306306
var option = new Option<int>("-x", () => 123);
307-
option.AddValidator( symbol =>
308-
symbol.Tokens
309-
.Select(t => t.Value)
310-
.Where(v => v == "123")
311-
.Select(x => "ERR")
312-
.FirstOrDefault());
313-
307+
option.AddValidator(symbol =>
308+
symbol.ErrorMessage = symbol.Tokens
309+
.Select(t => t.Value)
310+
.Where(v => v == "123")
311+
.Select(_ => "ERR")
312+
.FirstOrDefault());
314313

315314
option
316315
.Parse("-x 123")

‎src/System.CommandLine.Tests/ParsingValidationTests.cs

Lines changed: 6 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -242,10 +242,8 @@ public void A_custom_validator_can_be_added_to_a_command()
242242
if (commandResult.Children.Any(sr => sr.Symbol is IdentifierSymbol id && id.HasAlias("--one")) &&
243243
commandResult.Children.Any(sr => sr.Symbol is IdentifierSymbol id && id.HasAlias("--two")))
244244
{
245-
return "Options '--one' and '--two' cannot be used together.";
245+
commandResult.ErrorMessage = "Options '--one' and '--two' cannot be used together.";
246246
}
247-
248-
return null;
249247
});
250248

251249
var result = command.Parse("the-command --one --two");
@@ -266,7 +264,7 @@ public void A_custom_validator_can_be_added_to_an_option()
266264
{
267265
var value = r.GetValueOrDefault<int>();
268266

269-
return $"Option {r.Token.Value} cannot be set to {value}";
267+
r.ErrorMessage = $"Option {r.Token.Value} cannot be set to {value}";
270268
});
271269

272270
var command = new RootCommand { option };
@@ -291,7 +289,7 @@ public void A_custom_validator_can_be_added_to_an_argument()
291289
{
292290
var value = r.GetValueOrDefault<int>();
293291

294-
return $"Argument {r.Argument.Name} cannot be set to {value}";
292+
r.ErrorMessage = $"Argument {r.Argument.Name} cannot be set to {value}";
295293
});
296294

297295
var command = new RootCommand { argument };
@@ -320,14 +318,12 @@ public void All_custom_validators_are_called(string commandLine)
320318
option.AddValidator(_ =>
321319
{
322320
optionValidatorWasCalled = true;
323-
return null;
324321
});
325322

326323
var argument = new Argument<string>("the-arg");
327324
argument.AddValidator(_ =>
328325
{
329326
argumentValidatorWasCalled = true;
330-
return null;
331327
});
332328

333329
var rootCommand = new RootCommand
@@ -338,7 +334,6 @@ public void All_custom_validators_are_called(string commandLine)
338334
rootCommand.AddValidator(_ =>
339335
{
340336
commandValidatorWasCalled = true;
341-
return null;
342337
});
343338

344339
rootCommand.Invoke(commandLine);
@@ -356,7 +351,7 @@ public void Validators_on_global_options_are_executed_when_invoking_a_subcommand
356351
var option = new Option<FileInfo>("--file");
357352
option.AddValidator(r =>
358353
{
359-
return "Invoked validator";
354+
r.ErrorMessage = "Invoked validator";
360355
});
361356

362357
var subCommand = new Command("subcommand");
@@ -389,7 +384,7 @@ public async Task A_custom_validator_added_to_a_global_option_is_checked(string
389384
var handlerWasCalled = false;
390385

391386
var globalOption = new Option<int>("--value");
392-
globalOption.AddValidator(_ => "oops!");
387+
globalOption.AddValidator(r => r.ErrorMessage = "oops!");
393388

394389
var grandchildCommand = new Command("grandchild");
395390

@@ -419,7 +414,7 @@ public void Custom_validator_error_messages_are_not_repeated()
419414
{
420415
var errorMessage = "that's not right...";
421416
var argument = new Argument<string>();
422-
argument.AddValidator(o => errorMessage);
417+
argument.AddValidator(r => r.ErrorMessage = errorMessage);
423418

424419
var cmd = new Command("get")
425420
{

‎src/System.CommandLine/ArgumentExtensions.cs

Lines changed: 14 additions & 49 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,6 @@
44
using System.Collections.Generic;
55
using System.CommandLine.Parsing;
66
using System.CommandLine.Completions;
7-
using System.Linq;
87
using System.IO;
98

109
namespace System.CommandLine
@@ -90,12 +89,7 @@ public static TArgument FromAmong<TArgument>(
9089
/// <returns>The configured argument.</returns>
9190
public static Argument<FileInfo> ExistingOnly(this Argument<FileInfo> argument)
9291
{
93-
argument.AddValidator(symbol =>
94-
symbol.Tokens
95-
.Select(t => t.Value)
96-
.Where(filePath => !File.Exists(filePath))
97-
.Select(symbol.LocalizationResources.FileDoesNotExist)
98-
.FirstOrDefault());
92+
argument.AddValidator(Validate.FileExists);
9993
return argument;
10094
}
10195

@@ -106,12 +100,7 @@ public static Argument<FileInfo> ExistingOnly(this Argument<FileInfo> argument)
106100
/// <returns>The configured argument.</returns>
107101
public static Argument<DirectoryInfo> ExistingOnly(this Argument<DirectoryInfo> argument)
108102
{
109-
argument.AddValidator(symbol =>
110-
symbol.Tokens
111-
.Select(t => t.Value)
112-
.Where(filePath => !Directory.Exists(filePath))
113-
.Select(symbol.LocalizationResources.DirectoryDoesNotExist)
114-
.FirstOrDefault());
103+
argument.AddValidator(Validate.DirectoryExists);
115104
return argument;
116105
}
117106

@@ -122,12 +111,7 @@ public static Argument<DirectoryInfo> ExistingOnly(this Argument<DirectoryInfo>
122111
/// <returns>The configured argument.</returns>
123112
public static Argument<FileSystemInfo> ExistingOnly(this Argument<FileSystemInfo> argument)
124113
{
125-
argument.AddValidator(symbol =>
126-
symbol.Tokens
127-
.Select(t => t.Value)
128-
.Where(filePath => !Directory.Exists(filePath) && !File.Exists(filePath))
129-
.Select(symbol.LocalizationResources.FileOrDirectoryDoesNotExist)
130-
.FirstOrDefault());
114+
argument.AddValidator(Validate.FileOrDirectoryExists);
131115
return argument;
132116
}
133117

@@ -141,30 +125,15 @@ public static Argument<T> ExistingOnly<T>(this Argument<T> argument)
141125
{
142126
if (typeof(IEnumerable<FileInfo>).IsAssignableFrom(typeof(T)))
143127
{
144-
argument.AddValidator(
145-
a => a.Tokens
146-
.Select(t => t.Value)
147-
.Where(filePath => !File.Exists(filePath))
148-
.Select(a.LocalizationResources.FileDoesNotExist)
149-
.FirstOrDefault());
128+
argument.AddValidator(Validate.FileExists);
150129
}
151130
else if (typeof(IEnumerable<DirectoryInfo>).IsAssignableFrom(typeof(T)))
152131
{
153-
argument.AddValidator(
154-
a => a.Tokens
155-
.Select(t => t.Value)
156-
.Where(filePath => !Directory.Exists(filePath))
157-
.Select(a.LocalizationResources.DirectoryDoesNotExist)
158-
.FirstOrDefault());
132+
argument.AddValidator(Validate.DirectoryExists);
159133
}
160134
else
161135
{
162-
argument.AddValidator(
163-
a => a.Tokens
164-
.Select(t => t.Value)
165-
.Where(filePath => !Directory.Exists(filePath) && !File.Exists(filePath))
166-
.Select(a.LocalizationResources.FileOrDirectoryDoesNotExist)
167-
.FirstOrDefault());
136+
argument.AddValidator(Validate.FileOrDirectoryExists);
168137
}
169138

170139
return argument;
@@ -181,23 +150,21 @@ public static TArgument LegalFilePathsOnly<TArgument>(
181150
{
182151
var invalidPathChars = Path.GetInvalidPathChars();
183152

184-
argument.AddValidator(symbol =>
153+
argument.AddValidator(result =>
185154
{
186-
for (var i = 0; i < symbol.Tokens.Count; i++)
155+
for (var i = 0; i < result.Tokens.Count; i++)
187156
{
188-
var token = symbol.Tokens[i];
157+
var token = result.Tokens[i];
189158

190159
// File class no longer check invalid character
191160
// https://blogs.msdn.microsoft.com/jeremykuhne/2018/03/09/custom-directory-enumeration-in-net-core-2-1/
192161
var invalidCharactersIndex = token.Value.IndexOfAny(invalidPathChars);
193162

194163
if (invalidCharactersIndex >= 0)
195164
{
196-
return symbol.LocalizationResources.InvalidCharactersInPath(token.Value[invalidCharactersIndex]);
165+
result.ErrorMessage = result.LocalizationResources.InvalidCharactersInPath(token.Value[invalidCharactersIndex]);
197166
}
198167
}
199-
200-
return null;
201168
});
202169

203170
return argument;
@@ -215,20 +182,18 @@ public static TArgument LegalFileNamesOnly<TArgument>(
215182
{
216183
var invalidFileNameChars = Path.GetInvalidFileNameChars();
217184

218-
argument.AddValidator(symbol =>
185+
argument.AddValidator(result =>
219186
{
220-
for (var i = 0; i < symbol.Tokens.Count; i++)
187+
for (var i = 0; i < result.Tokens.Count; i++)
221188
{
222-
var token = symbol.Tokens[i];
189+
var token = result.Tokens[i];
223190
var invalidCharactersIndex = token.Value.IndexOfAny(invalidFileNameChars);
224191

225192
if (invalidCharactersIndex >= 0)
226193
{
227-
return symbol.LocalizationResources.InvalidCharactersInFileName(token.Value[invalidCharactersIndex]);
194+
result.ErrorMessage = result.LocalizationResources.InvalidCharactersInFileName(token.Value[invalidCharactersIndex]);
228195
}
229196
}
230-
231-
return null;
232197
});
233198

234199
return argument;

‎src/System.CommandLine/Help/VersionOption.cs

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -38,10 +38,8 @@ private void AddValidators()
3838
parent.Children.Where(r => r.Symbol is not VersionOption)
3939
.Any(IsNotImplicit))
4040
{
41-
return result.LocalizationResources.VersionOptionCannotBeCombinedWithOtherArguments(result.Token.Value ?? result.Symbol.Name);
41+
result.ErrorMessage = result.LocalizationResources.VersionOptionCannotBeCombinedWithOtherArguments(result.Token.Value ?? result.Symbol.Name);
4242
}
43-
44-
return null;
4543
});
4644
}
4745

‎src/System.CommandLine/Option.cs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -298,6 +298,7 @@ private string GetLongestAlias()
298298
return max.RemovePrefix();
299299
}
300300

301+
/// <inheritdoc />
301302
public override IEnumerable<CompletionItem> GetCompletions(CompletionContext context)
302303
{
303304
if (_argument is null)

‎src/System.CommandLine/OptionExtensions.cs

Lines changed: 3 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -90,14 +90,7 @@ public static TOption AddCompletions<TOption>(
9090
/// <returns>The option being extended.</returns>
9191
public static Option<FileInfo> ExistingOnly(this Option<FileInfo> option)
9292
{
93-
option.Argument.AddValidator(
94-
a =>
95-
a.Tokens
96-
.Select(t => t.Value)
97-
.Where(filePath => !File.Exists(filePath))
98-
.Select(a.LocalizationResources.FileDoesNotExist)
99-
.FirstOrDefault());
100-
93+
option.Argument.AddValidator(Validate.FileExists);
10194
return option;
10295
}
10396

@@ -108,14 +101,7 @@ public static Option<FileInfo> ExistingOnly(this Option<FileInfo> option)
108101
/// <returns>The option being extended.</returns>
109102
public static Option<DirectoryInfo> ExistingOnly(this Option<DirectoryInfo> option)
110103
{
111-
option.Argument.AddValidator(
112-
a =>
113-
a.Tokens
114-
.Select(t => t.Value)
115-
.Where(filePath => !Directory.Exists(filePath))
116-
.Select(a.LocalizationResources.DirectoryDoesNotExist)
117-
.FirstOrDefault());
118-
104+
option.Argument.AddValidator(Validate.DirectoryExists);
119105
return option;
120106
}
121107

@@ -126,14 +112,7 @@ public static Option<DirectoryInfo> ExistingOnly(this Option<DirectoryInfo> opti
126112
/// <returns>The option being extended.</returns>
127113
public static Option<FileSystemInfo> ExistingOnly(this Option<FileSystemInfo> option)
128114
{
129-
option.Argument.AddValidator(
130-
a =>
131-
a.Tokens
132-
.Select(t => t.Value)
133-
.Where(filePath => !Directory.Exists(filePath) && !File.Exists(filePath))
134-
.Select(a.LocalizationResources.FileOrDirectoryDoesNotExist)
135-
.FirstOrDefault());
136-
115+
option.Argument.AddValidator(Validate.FileOrDirectoryExists);
137116
return option;
138117
}
139118

‎src/System.CommandLine/Parsing/ArgumentResult.cs

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -90,12 +90,12 @@ public void OnlyTake(int numberOfTokens)
9090

9191
for (var i = 0; i < argument.Validators.Count; i++)
9292
{
93-
var symbolValidator = argument.Validators[i];
94-
var errorMessage = symbolValidator(this);
93+
var validateSymbolResult = argument.Validators[i];
94+
validateSymbolResult(this);
9595

96-
if (!string.IsNullOrWhiteSpace(errorMessage))
96+
if (!string.IsNullOrWhiteSpace(ErrorMessage))
9797
{
98-
return new ParseError(errorMessage!, this);
98+
return new ParseError(ErrorMessage!, this);
9999
}
100100
}
101101

‎src/System.CommandLine/Parsing/ParseResultVisitor.cs

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -325,12 +325,12 @@ private void ValidateCommandResult()
325325

326326
for (var i = 0; i < command.Validators.Count; i++)
327327
{
328-
var validator = command.Validators[i];
329-
var errorMessage = validator(_innermostCommandResult);
328+
var validateSymbolResult = command.Validators[i];
329+
validateSymbolResult(_innermostCommandResult);
330330

331-
if (!string.IsNullOrWhiteSpace(errorMessage))
331+
if (!string.IsNullOrWhiteSpace(_innermostCommandResult.ErrorMessage))
332332
{
333-
AddErrorToResult(_innermostCommandResult, new ParseError(errorMessage!, _innermostCommandResult));
333+
AddErrorToResult(_innermostCommandResult, new ParseError(_innermostCommandResult.ErrorMessage!, _innermostCommandResult));
334334
}
335335
}
336336

@@ -440,11 +440,11 @@ private void ValidateAndConvertOptionResult(OptionResult optionResult)
440440
for (var i = 0; i < optionResult.Option.Validators.Count; i++)
441441
{
442442
var validate = optionResult.Option.Validators[i];
443-
var message = validate(optionResult);
443+
validate(optionResult);
444444

445-
if (!string.IsNullOrWhiteSpace(message))
445+
if (!string.IsNullOrWhiteSpace(optionResult.ErrorMessage))
446446
{
447-
AddErrorToResult(optionResult, new ParseError(message!, optionResult));
447+
AddErrorToResult(optionResult, new ParseError(optionResult.ErrorMessage!, optionResult));
448448
}
449449
}
450450
}

‎src/System.CommandLine/Parsing/ValidateSymbolResult.cs

Lines changed: 10 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,13 @@
11
// Copyright (c) .NET Foundation and contributors. All rights reserved.
22
// Licensed under the MIT license. See LICENSE file in the project root for full license information.
33

4-
namespace System.CommandLine.Parsing
5-
{
6-
/// <summary>
7-
/// A delegate used to validate symbol results during parsing.
8-
/// </summary>
9-
/// <typeparam name="T">The type of the <see cref="SymbolResult"/>.</typeparam>
10-
/// <param name="symbolResult">The symbol result</param>
11-
/// <returns>An error message to indicate a validation error; otherwise, <c>null</c>.</returns>
12-
public delegate string? ValidateSymbolResult<in T>(T symbolResult)
13-
where T : SymbolResult;
14-
}
4+
namespace System.CommandLine.Parsing;
5+
6+
/// <summary>
7+
/// A delegate used to validate symbol results during parsing.
8+
/// </summary>
9+
/// <typeparam name="T">The type of the <see cref="SymbolResult"/>.</typeparam>
10+
/// <param name="symbolResult">The symbol result</param>
11+
/// <remarks>To display an error, set <see cref="SymbolResult.ErrorMessage"/>.</remarks>
12+
public delegate void ValidateSymbolResult<in T>(T symbolResult)
13+
where T : SymbolResult;

0 commit comments

Comments
 (0)