Skip to content

Commit 4db05a0

Browse files
authored
Fix false config validation warnings for process selectors (#6542)
Signed-off-by: Ben Sherman <[email protected]>
1 parent 9e296b8 commit 4db05a0

File tree

2 files changed

+94
-56
lines changed

2 files changed

+94
-56
lines changed

modules/nextflow/src/main/groovy/nextflow/config/ConfigValidator.groovy

Lines changed: 52 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -89,35 +89,35 @@ class ConfigValidator {
8989
pluginScopes = new SpecNode.Scope('', children)
9090
}
9191

92-
void validate(ConfigMap config) {
93-
validate(config.toConfigObject())
94-
}
92+
/**
93+
* Validate a config block within the given scope.
94+
*
95+
* @param config
96+
* @param scopes
97+
*/
98+
void validate(Map<String,?> config, List<String> scopes=[]) {
99+
for( final entry : config.entrySet() ) {
100+
final key = entry.key
101+
final value = entry.value
102+
103+
final names = scopes + [key]
104+
105+
if( names.size() == 2 && names.first() == 'profiles' )
106+
names.clear()
95107

96-
void validate(ConfigObject config) {
97-
final flatConfig = config.flatten()
98-
for( String key : flatConfig.keySet() ) {
99-
final names = key.tokenize('.').findAll { name -> !isSelector(name) }
100-
if( names.first() == 'profiles' ) {
101-
if( !names.isEmpty() ) names.remove(0)
102-
if( !names.isEmpty() ) names.remove(0)
108+
if( value instanceof Map ) {
109+
log.debug "validate config block ${names}"
110+
if( isSelector(key) )
111+
names.removeLast()
112+
log.debug " is map option ${isMapOption(names)}"
113+
if( isMapOption(names) )
114+
continue
115+
validate(value, names)
103116
}
104-
final scope = names.first()
105-
if( scope == 'env' ) {
106-
checkEnv(names.last())
107-
continue
117+
else {
118+
log.debug "validate config option ${names}"
119+
validateOption(names)
108120
}
109-
if( scope == 'params' )
110-
continue
111-
final fqName = names.join('.')
112-
if( fqName.startsWith('process.ext.') )
113-
continue
114-
if( isValid(names) )
115-
continue
116-
if( isMissingCorePluginScope(names.first()) )
117-
continue
118-
if( isMapOption(names) )
119-
continue
120-
log.warn1 "Unrecognized config option '${fqName}'"
121121
}
122122
}
123123

@@ -130,6 +130,29 @@ class ConfigValidator {
130130
return name.startsWith('withLabel:') || name.startsWith('withName:')
131131
}
132132

133+
/**
134+
* Validate a config option given by the list of names.
135+
*
136+
* For example, the option 'process.resourceLimits' is represented
137+
* as ['process', 'resourceLimits'].
138+
*
139+
* @param names
140+
*/
141+
void validateOption(List<String> names) {
142+
final scope = names.first()
143+
if( scope == 'env' ) {
144+
checkEnv(names.last())
145+
return
146+
}
147+
if( scope == 'params' )
148+
return
149+
if( isMissingCorePluginScope(scope) )
150+
return
151+
if( isValid(names) )
152+
return
153+
log.warn1 "Unrecognized config option '${names.join('.')}'"
154+
}
155+
133156
/**
134157
* Determine whether a config option is defined in the spec.
135158
*
@@ -161,26 +184,16 @@ class ConfigValidator {
161184
* Determine whether a config option is a map option or a
162185
* property thereof.
163186
*
164-
* @param names Config option split into individual names, e.g. 'process.resourceLimits' -> [process, resourceLimits]
187+
* @param names
165188
*/
166189
private boolean isMapOption(List<String> names) {
167190
return isMapOption0(SpecNode.ROOT, names)
168191
|| isMapOption0(pluginScopes, names)
169192
}
170193

171194
private static boolean isMapOption0(SpecNode.Scope scope, List<String> names) {
172-
SpecNode node = scope
173-
for( final name : names ) {
174-
if( node instanceof SpecNode.Scope )
175-
node = node.children().get(name)
176-
else if( node instanceof SpecNode.Placeholder )
177-
node = node.scope()
178-
else if( node instanceof SpecNode.Option )
179-
return node.type() == Map.class
180-
else
181-
return false
182-
}
183-
return false
195+
final node = scope.getOption(names)
196+
return node != null && node.type() == Map.class
184197
}
185198

186199
/**

modules/nextflow/src/test/groovy/nextflow/config/ConfigValidatorTest.groovy

Lines changed: 42 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -29,34 +29,62 @@ class ConfigValidatorTest extends Specification {
2929
OutputCapture capture = new OutputCapture()
3030

3131
def 'should warn about invalid config options' () {
32-
given:
33-
def config = new ConfigMap([
32+
when:
33+
new ConfigValidator().validate([
3434
wokDir: 'work',
3535
workDir: 'work',
3636
process: [
3737
cpu: 2,
3838
cpus: 2
3939
]
4040
])
41-
42-
when:
43-
new ConfigValidator().validate(config)
4441
then:
4542
capture.toString().contains('Unrecognized config option \'wokDir\'')
4643
capture.toString().contains('Unrecognized config option \'process.cpu\'')
4744
!capture.toString().contains('Unrecognized config option \'workDir\'')
4845
!capture.toString().contains('Unrecognized config option \'process.cpus\'')
4946
}
5047

48+
def 'should validate config options in profiles' () {
49+
when:
50+
new ConfigValidator().validate([
51+
profiles: [
52+
test: [
53+
workDir: 'work',
54+
process: [
55+
cpus: 2
56+
]
57+
]
58+
]
59+
])
60+
then:
61+
!capture.toString().contains('Unrecognized config option')
62+
63+
when:
64+
new ConfigValidator().validate([
65+
profiles: [
66+
test: [
67+
wokDir2: 'work',
68+
process: [
69+
cpu2: 2
70+
]
71+
]
72+
]
73+
])
74+
then:
75+
capture.toString().contains('Unrecognized config option \'wokDir2\'')
76+
capture.toString().contains('Unrecognized config option \'process.cpu2\'')
77+
}
78+
5179
def 'should warn about invalid env config options' () {
5280
when:
53-
new ConfigValidator().validate(new ConfigMap([
81+
new ConfigValidator().validate([
5482
env: [
5583
FOO: '/something',
5684
NXF_ANSI_SUMMARY: 'true',
5785
NXF_DEBUG: 'true'
5886
]
59-
]))
87+
])
6088

6189
then:
6290
capture.toString().contains('the following environment variable in the config will be ignored: \'NXF_ANSI_SUMMARY\'')
@@ -65,27 +93,27 @@ class ConfigValidatorTest extends Specification {
6593
}
6694

6795
def 'should ignore process selectors' () {
68-
given:
69-
def config = new ConfigMap([
96+
when:
97+
new ConfigValidator().validate([
7098
process: [
7199
'withLabel:foobar': [
72100
cpus: 2
73101
],
74102
'withName:foobar': [
75103
cpus: 2
104+
],
105+
"withName:'.*TASK.*'": [
106+
cpus: 2
76107
]
77108
]
78109
])
79-
80-
when:
81-
new ConfigValidator().validate(config)
82110
then:
83111
!capture.toString().contains('Unrecognized config option')
84112
}
85113

86114
def 'should support map options' () {
87-
given:
88-
def config = new ConfigMap([
115+
when:
116+
new ConfigValidator().validate([
89117
process: [
90118
resourceLimits: [
91119
cpus: 4,
@@ -94,9 +122,6 @@ class ConfigValidatorTest extends Specification {
94122
]
95123
]
96124
])
97-
98-
when:
99-
new ConfigValidator().validate(config)
100125
then:
101126
!capture.toString().contains('Unrecognized config option')
102127
}

0 commit comments

Comments
 (0)