Skip to content

Commit b200a86

Browse files
epageepage
authored
Merge pull request #3288 from epage/assert
fix: Run more parse asserts during build
2 parents 37f47dd + de275e7 commit b200a86

File tree

2 files changed

+171
-178
lines changed

2 files changed

+171
-178
lines changed

‎src/build/app/debug_asserts.rs‎

Lines changed: 171 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,8 @@
11
use crate::{
22
build::arg::{debug_asserts::assert_arg, ArgProvider},
3+
mkeymap::KeyType,
34
util::Id,
4-
App, AppSettings, ArgSettings, ValueHint,
5+
App, AppSettings, Arg, ArgSettings, ValueHint,
56
};
67
use std::cmp::Ordering;
78

@@ -274,6 +275,8 @@ pub(crate) fn assert_app(app: &App) {
274275
detect_duplicate_flags(&long_flags, "long");
275276
detect_duplicate_flags(&short_flags, "short");
276277

278+
_verify_positionals(app);
279+
277280
if let Some(help_template) = app.template {
278281
assert!(
279282
!help_template.contains("{flags}"),
@@ -410,3 +413,170 @@ fn assert_app_flags(app: &App) {
410413
#[cfg(feature = "unstable-multicall")]
411414
checker!(Multicall conflicts NoBinaryName);
412415
}
416+
417+
#[cfg(debug_assertions)]
418+
fn _verify_positionals(app: &App) -> bool {
419+
debug!("App::_verify_positionals");
420+
// Because you must wait until all arguments have been supplied, this is the first chance
421+
// to make assertions on positional argument indexes
422+
//
423+
// First we verify that the index highest supplied index, is equal to the number of
424+
// positional arguments to verify there are no gaps (i.e. supplying an index of 1 and 3
425+
// but no 2)
426+
427+
let highest_idx = app
428+
.args
429+
.keys()
430+
.filter_map(|x| {
431+
if let KeyType::Position(n) = x {
432+
Some(*n)
433+
} else {
434+
None
435+
}
436+
})
437+
.max()
438+
.unwrap_or(0);
439+
440+
let num_p = app.args.keys().filter(|x| x.is_position()).count();
441+
442+
assert!(
443+
highest_idx == num_p,
444+
"Found positional argument whose index is {} but there \
445+
are only {} positional arguments defined",
446+
highest_idx,
447+
num_p
448+
);
449+
450+
// Next we verify that only the highest index has takes multiple arguments (if any)
451+
let only_highest = |a: &Arg| a.is_multiple() && (a.index.unwrap_or(0) != highest_idx);
452+
if app.get_positionals().any(only_highest) {
453+
// First we make sure if there is a positional that allows multiple values
454+
// the one before it (second to last) has one of these:
455+
// * a value terminator
456+
// * ArgSettings::Last
457+
// * The last arg is Required
458+
459+
// We can't pass the closure (it.next()) to the macro directly because each call to
460+
// find() (iterator, not macro) gets called repeatedly.
461+
let last = &app.args[&KeyType::Position(highest_idx)];
462+
let second_to_last = &app.args[&KeyType::Position(highest_idx - 1)];
463+
464+
// Either the final positional is required
465+
// Or the second to last has a terminator or .last(true) set
466+
let ok = last.is_set(ArgSettings::Required)
467+
|| (second_to_last.terminator.is_some() || second_to_last.is_set(ArgSettings::Last))
468+
|| last.is_set(ArgSettings::Last);
469+
assert!(
470+
ok,
471+
"When using a positional argument with .multiple_values(true) that is *not the \
472+
last* positional argument, the last positional argument (i.e. the one \
473+
with the highest index) *must* have .required(true) or .last(true) set."
474+
);
475+
476+
// We make sure if the second to last is Multiple the last is ArgSettings::Last
477+
let ok = second_to_last.is_multiple() || last.is_set(ArgSettings::Last);
478+
assert!(
479+
ok,
480+
"Only the last positional argument, or second to last positional \
481+
argument may be set to .multiple_values(true)"
482+
);
483+
484+
// Next we check how many have both Multiple and not a specific number of values set
485+
let count = app
486+
.get_positionals()
487+
.filter(|p| {
488+
p.settings.is_set(ArgSettings::MultipleOccurrences)
489+
|| (p.settings.is_set(ArgSettings::MultipleValues) && p.num_vals.is_none())
490+
})
491+
.count();
492+
let ok = count <= 1
493+
|| (last.is_set(ArgSettings::Last)
494+
&& last.is_multiple()
495+
&& second_to_last.is_multiple()
496+
&& count == 2);
497+
assert!(
498+
ok,
499+
"Only one positional argument with .multiple_values(true) set is allowed per \
500+
command, unless the second one also has .last(true) set"
501+
);
502+
}
503+
504+
let mut found = false;
505+
506+
if app.is_set(AppSettings::AllowMissingPositional) {
507+
// Check that if a required positional argument is found, all positions with a lower
508+
// index are also required.
509+
let mut foundx2 = false;
510+
511+
for p in app.get_positionals() {
512+
if foundx2 && !p.is_set(ArgSettings::Required) {
513+
assert!(
514+
p.is_set(ArgSettings::Required),
515+
"Found non-required positional argument with a lower \
516+
index than a required positional argument by two or more: {:?} \
517+
index {:?}",
518+
p.name,
519+
p.index
520+
);
521+
} else if p.is_set(ArgSettings::Required) && !p.is_set(ArgSettings::Last) {
522+
// Args that .last(true) don't count since they can be required and have
523+
// positionals with a lower index that aren't required
524+
// Imagine: prog <req1> [opt1] -- <req2>
525+
// Both of these are valid invocations:
526+
// $ prog r1 -- r2
527+
// $ prog r1 o1 -- r2
528+
if found {
529+
foundx2 = true;
530+
continue;
531+
}
532+
found = true;
533+
continue;
534+
} else {
535+
found = false;
536+
}
537+
}
538+
} else {
539+
// Check that if a required positional argument is found, all positions with a lower
540+
// index are also required
541+
for p in (1..=num_p).rev().filter_map(|n| app.args.get(&n)) {
542+
if found {
543+
assert!(
544+
p.is_set(ArgSettings::Required),
545+
"Found non-required positional argument with a lower \
546+
index than a required positional argument: {:?} index {:?}",
547+
p.name,
548+
p.index
549+
);
550+
} else if p.is_set(ArgSettings::Required) && !p.is_set(ArgSettings::Last) {
551+
// Args that .last(true) don't count since they can be required and have
552+
// positionals with a lower index that aren't required
553+
// Imagine: prog <req1> [opt1] -- <req2>
554+
// Both of these are valid invocations:
555+
// $ prog r1 -- r2
556+
// $ prog r1 o1 -- r2
557+
found = true;
558+
continue;
559+
}
560+
}
561+
}
562+
assert!(
563+
app.get_positionals()
564+
.filter(|p| p.is_set(ArgSettings::Last))
565+
.count()
566+
< 2,
567+
"Only one positional argument may have last(true) set. Found two."
568+
);
569+
if app
570+
.get_positionals()
571+
.any(|p| p.is_set(ArgSettings::Last) && p.is_set(ArgSettings::Required))
572+
&& app.has_subcommands()
573+
&& !app.is_set(AppSettings::SubcommandsNegateReqs)
574+
{
575+
panic!(
576+
"Having a required positional argument with .last(true) set *and* child \
577+
subcommands without setting SubcommandsNegateReqs isn't compatible."
578+
);
579+
}
580+
581+
true
582+
}

‎src/parse/parser.rs‎

Lines changed: 0 additions & 177 deletions
Original file line numberDiff line numberDiff line change
@@ -63,9 +63,6 @@ impl<'help, 'app> Parser<'help, 'app> {
6363
pub(crate) fn _build(&mut self) {
6464
debug!("Parser::_build");
6565

66-
#[cfg(debug_assertions)]
67-
self._verify_positionals();
68-
6966
for group in &self.app.groups {
7067
if group.required {
7168
let idx = self.required.insert(group.id.clone());
@@ -76,180 +73,6 @@ impl<'help, 'app> Parser<'help, 'app> {
7673
}
7774
}
7875

79-
#[cfg(debug_assertions)]
80-
fn _verify_positionals(&self) -> bool {
81-
debug!("Parser::_verify_positionals");
82-
// Because you must wait until all arguments have been supplied, this is the first chance
83-
// to make assertions on positional argument indexes
84-
//
85-
// First we verify that the index highest supplied index, is equal to the number of
86-
// positional arguments to verify there are no gaps (i.e. supplying an index of 1 and 3
87-
// but no 2)
88-
89-
let highest_idx = self
90-
.app
91-
.args
92-
.keys()
93-
.filter_map(|x| {
94-
if let KeyType::Position(n) = x {
95-
Some(*n)
96-
} else {
97-
None
98-
}
99-
})
100-
.max()
101-
.unwrap_or(0);
102-
103-
//_highest_idx(&self.positionals);
104-
105-
let num_p = self.app.args.keys().filter(|x| x.is_position()).count();
106-
107-
assert!(
108-
highest_idx == num_p,
109-
"Found positional argument whose index is {} but there \
110-
are only {} positional arguments defined",
111-
highest_idx,
112-
num_p
113-
);
114-
115-
// Next we verify that only the highest index has takes multiple arguments (if any)
116-
let only_highest = |a: &Arg| a.is_multiple() && (a.index.unwrap_or(0) != highest_idx);
117-
if self.app.get_positionals().any(only_highest) {
118-
// First we make sure if there is a positional that allows multiple values
119-
// the one before it (second to last) has one of these:
120-
// * a value terminator
121-
// * ArgSettings::Last
122-
// * The last arg is Required
123-
124-
// We can't pass the closure (it.next()) to the macro directly because each call to
125-
// find() (iterator, not macro) gets called repeatedly.
126-
let last = &self.app.args[&KeyType::Position(highest_idx)];
127-
let second_to_last = &self.app.args[&KeyType::Position(highest_idx - 1)];
128-
129-
// Either the final positional is required
130-
// Or the second to last has a terminator or .last(true) set
131-
let ok = last.is_set(ArgSettings::Required)
132-
|| (second_to_last.terminator.is_some()
133-
|| second_to_last.is_set(ArgSettings::Last))
134-
|| last.is_set(ArgSettings::Last);
135-
assert!(
136-
ok,
137-
"When using a positional argument with .multiple_values(true) that is *not the \
138-
last* positional argument, the last positional argument (i.e. the one \
139-
with the highest index) *must* have .required(true) or .last(true) set."
140-
);
141-
142-
// We make sure if the second to last is Multiple the last is ArgSettings::Last
143-
let ok = second_to_last.is_multiple() || last.is_set(ArgSettings::Last);
144-
assert!(
145-
ok,
146-
"Only the last positional argument, or second to last positional \
147-
argument may be set to .multiple_values(true)"
148-
);
149-
150-
// Next we check how many have both Multiple and not a specific number of values set
151-
let count = self
152-
.app
153-
.get_positionals()
154-
.filter(|p| {
155-
p.settings.is_set(ArgSettings::MultipleOccurrences)
156-
|| (p.settings.is_set(ArgSettings::MultipleValues) && p.num_vals.is_none())
157-
})
158-
.count();
159-
let ok = count <= 1
160-
|| (last.is_set(ArgSettings::Last)
161-
&& last.is_multiple()
162-
&& second_to_last.is_multiple()
163-
&& count == 2);
164-
assert!(
165-
ok,
166-
"Only one positional argument with .multiple_values(true) set is allowed per \
167-
command, unless the second one also has .last(true) set"
168-
);
169-
}
170-
171-
let mut found = false;
172-
173-
if self.is_set(AS::AllowMissingPositional) {
174-
// Check that if a required positional argument is found, all positions with a lower
175-
// index are also required.
176-
let mut foundx2 = false;
177-
178-
for p in self.app.get_positionals() {
179-
if foundx2 && !p.is_set(ArgSettings::Required) {
180-
assert!(
181-
p.is_set(ArgSettings::Required),
182-
"Found non-required positional argument with a lower \
183-
index than a required positional argument by two or more: {:?} \
184-
index {:?}",
185-
p.name,
186-
p.index
187-
);
188-
} else if p.is_set(ArgSettings::Required) && !p.is_set(ArgSettings::Last) {
189-
// Args that .last(true) don't count since they can be required and have
190-
// positionals with a lower index that aren't required
191-
// Imagine: prog <req1> [opt1] -- <req2>
192-
// Both of these are valid invocations:
193-
// $ prog r1 -- r2
194-
// $ prog r1 o1 -- r2
195-
if found {
196-
foundx2 = true;
197-
continue;
198-
}
199-
found = true;
200-
continue;
201-
} else {
202-
found = false;
203-
}
204-
}
205-
} else {
206-
// Check that if a required positional argument is found, all positions with a lower
207-
// index are also required
208-
for p in (1..=num_p).rev().filter_map(|n| self.app.args.get(&n)) {
209-
if found {
210-
assert!(
211-
p.is_set(ArgSettings::Required),
212-
"Found non-required positional argument with a lower \
213-
index than a required positional argument: {:?} index {:?}",
214-
p.name,
215-
p.index
216-
);
217-
} else if p.is_set(ArgSettings::Required) && !p.is_set(ArgSettings::Last) {
218-
// Args that .last(true) don't count since they can be required and have
219-
// positionals with a lower index that aren't required
220-
// Imagine: prog <req1> [opt1] -- <req2>
221-
// Both of these are valid invocations:
222-
// $ prog r1 -- r2
223-
// $ prog r1 o1 -- r2
224-
found = true;
225-
continue;
226-
}
227-
}
228-
}
229-
assert!(
230-
self.app
231-
.get_positionals()
232-
.filter(|p| p.is_set(ArgSettings::Last))
233-
.count()
234-
< 2,
235-
"Only one positional argument may have last(true) set. Found two."
236-
);
237-
if self
238-
.app
239-
.get_positionals()
240-
.any(|p| p.is_set(ArgSettings::Last) && p.is_set(ArgSettings::Required))
241-
&& self.app.has_subcommands()
242-
&& !self.is_set(AS::SubcommandsNegateReqs)
243-
{
244-
panic!(
245-
"Having a required positional argument with .last(true) set *and* child \
246-
subcommands without setting SubcommandsNegateReqs isn't compatible."
247-
);
248-
}
249-
250-
true
251-
}
252-
25376
// Should we color the help?
25477
pub(crate) fn color_help(&self) -> ColorChoice {
25578
#[cfg(feature = "color")]

0 commit comments

Comments
 (0)