Skip to content

Commit fa17f2d

Browse files
mpkorstanjempkorstanje
committed
[Core] Fix NPE when parsing empty feature file
1 parent b5a05d1 commit fa17f2d

File tree

24 files changed

+200
-52
lines changed

24 files changed

+200
-52
lines changed

Diff for: ‎core/src/main/java/io/cucumber/core/feature/FeatureParser.java

+2-1
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111
import java.util.Comparator;
1212
import java.util.Iterator;
1313
import java.util.List;
14+
import java.util.Optional;
1415
import java.util.ServiceLoader;
1516
import java.util.UUID;
1617
import java.util.function.Supplier;
@@ -27,7 +28,7 @@ public FeatureParser(Supplier<UUID> idGenerator) {
2728
}
2829

2930

30-
public Feature parseResource(Resource resource) {
31+
public Optional<Feature> parseResource(Resource resource) {
3132
requireNonNull(resource);
3233
URI uri = resource.getUri();
3334
String source = read(resource);

Diff for: ‎core/src/main/java/io/cucumber/core/plugin/JUnitFormatter.java

+4-3
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@
22

33
import io.cucumber.core.exception.CucumberException;
44
import io.cucumber.core.feature.FeatureParser;
5-
import io.cucumber.core.gherkin.Feature;
65
import io.cucumber.plugin.EventListener;
76
import io.cucumber.plugin.StrictAware;
87
import io.cucumber.plugin.event.EventPublisher;
@@ -106,8 +105,10 @@ public void setStrict(boolean strict) {
106105
}
107106

108107
private void handleTestSourceRead(TestSourceRead event) {
109-
Feature feature = parser.parseResource(new TestSourceReadResource(event));
110-
featuresNames.put(feature.getUri(), feature.getName());
108+
TestSourceReadResource source = new TestSourceReadResource(event);
109+
parser.parseResource(source).ifPresent(feature ->
110+
featuresNames.put(feature.getUri(), feature.getName())
111+
);
111112
}
112113

113114
private void handleTestCaseStarted(TestCaseStarted event) {

Diff for: ‎core/src/main/java/io/cucumber/core/plugin/TeamCityPlugin.java

+4-1
Original file line numberDiff line numberDiff line change
@@ -103,7 +103,10 @@ public void setEventPublisher(EventPublisher publisher) {
103103
private final FeatureParser featureParser = new FeatureParser(UUID::randomUUID);
104104

105105
private void handleTestSourceRead(TestSourceRead event) {
106-
features.put(event.getUri(), featureParser.parseResource(new TestSourceReadResource(event)));
106+
TestSourceReadResource source = new TestSourceReadResource(event);
107+
featureParser.parseResource(source).ifPresent(feature ->
108+
features.put(event.getUri(), feature)
109+
);
107110
}
108111

109112
private void printTestRunStarted(TestRunStarted event) {

Diff for: ‎core/src/main/java/io/cucumber/core/plugin/TestNGFormatter.java

+4-3
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@
22

33
import io.cucumber.core.exception.CucumberException;
44
import io.cucumber.core.feature.FeatureParser;
5-
import io.cucumber.core.gherkin.Feature;
65
import io.cucumber.plugin.EventListener;
76
import io.cucumber.plugin.StrictAware;
87
import io.cucumber.plugin.event.EventPublisher;
@@ -102,8 +101,10 @@ public void setStrict(boolean strict) {
102101
}
103102

104103
private void handleTestSourceRead(TestSourceRead event) {
105-
Feature feature = parser.parseResource(new TestSourceReadResource(event));
106-
featuresNames.put(feature.getUri(), feature.getName());
104+
TestSourceReadResource source = new TestSourceReadResource(event);
105+
parser.parseResource(source).ifPresent(feature ->
106+
featuresNames.put(feature.getUri(), feature.getName())
107+
);
107108
}
108109

109110
private void handleTestCaseStarted(TestCaseStarted event) {

Diff for: ‎core/src/main/java/io/cucumber/core/plugin/TimelineFormatter.java

+4-3
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,6 @@
55
import gherkin.deps.com.google.gson.annotations.SerializedName;
66
import io.cucumber.core.exception.CucumberException;
77
import io.cucumber.core.feature.FeatureParser;
8-
import io.cucumber.core.gherkin.Feature;
98
import io.cucumber.plugin.ConcurrentEventListener;
109
import io.cucumber.plugin.event.EventPublisher;
1110
import io.cucumber.plugin.event.TestCase;
@@ -76,8 +75,10 @@ public void setEventPublisher(final EventPublisher publisher) {
7675
}
7776

7877
private void handleTestSourceRead(TestSourceRead event) {
79-
Feature feature = parser.parseResource(new TestSourceReadResource(event));
80-
featuresNames.put(feature.getUri(), feature.getName());
78+
TestSourceReadResource source = new TestSourceReadResource(event);
79+
parser.parseResource(source).ifPresent(feature ->
80+
featuresNames.put(feature.getUri(), feature.getName())
81+
);
8182
}
8283

8384
private void handleTestCaseStarted(final TestCaseStarted event) {

Diff for: ‎core/src/main/java/io/cucumber/core/runtime/FeaturePathFeatureSupplier.java

+1-1
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,7 @@ public FeaturePathFeatureSupplier(Supplier<ClassLoader> classLoader, Options fea
3636
this.featureScanner = new ResourceScanner<>(
3737
classLoader,
3838
FeatureIdentifier::isFeature,
39-
resource -> of(parser.parseResource(resource))
39+
resource -> parser.parseResource(resource)
4040
);
4141
}
4242

Diff for: ‎core/src/test/java/io/cucumber/core/feature/TestFeatureParser.java

+1-1
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,6 @@ public InputStream getInputStream() {
3030
return new ByteArrayInputStream(source.getBytes(StandardCharsets.UTF_8));
3131
}
3232

33-
});
33+
}).orElse(null);
3434
}
3535
}

Diff for: ‎core/src/test/java/io/cucumber/core/runtime/FeatureBuilderTest.java

+2-2
Original file line numberDiff line numberDiff line change
@@ -91,9 +91,9 @@ public URI getUri() {
9191

9292
@Override
9393
public InputStream getInputStream() {
94-
return new ByteArrayInputStream("Feature: Example".getBytes(UTF_8));
94+
return new ByteArrayInputStream("Feature: Example\n Scenario: Empty".getBytes(UTF_8));
9595
}
96-
});
96+
}).orElse(null);
9797
}
9898

9999
}

Diff for: ‎gherkin-messages/pom.xml

+6
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,12 @@
3232
<artifactId>messages</artifactId>
3333
</dependency>
3434

35+
<dependency>
36+
<groupId>org.junit.jupiter</groupId>
37+
<artifactId>junit-jupiter</artifactId>
38+
<scope>test</scope>
39+
</dependency>
40+
3541
</dependencies>
3642

3743
<build>

Diff for: ‎gherkin-messages/src/main/java/io/cucumber/core/gherkin/messages/GherkinMessagesFeatureParser.java

+40-20
Original file line numberDiff line numberDiff line change
@@ -9,11 +9,13 @@
99
import io.cucumber.gherkin.GherkinDialectProvider;
1010
import io.cucumber.gherkin.ParserException;
1111
import io.cucumber.messages.Messages;
12+
import io.cucumber.messages.Messages.Envelope;
1213
import io.cucumber.messages.Messages.GherkinDocument;
1314

1415
import java.net.URI;
1516
import java.util.ArrayList;
1617
import java.util.List;
18+
import java.util.Optional;
1719
import java.util.UUID;
1820
import java.util.function.Supplier;
1921

@@ -24,40 +26,58 @@
2426
public final class GherkinMessagesFeatureParser implements FeatureParser {
2527

2628
@Override
27-
public Feature parse(URI path, String source, Supplier<UUID> idGenerator) {
29+
public Optional<Feature> parse(URI path, String source, Supplier<UUID> idGenerator) {
2830
try {
29-
CucumberQuery cucumberQuery = new CucumberQuery();
3031

31-
List<Messages.Envelope> sources = singletonList(
32+
List<Envelope> sources = singletonList(
3233
makeSourceEnvelope(source, path.toString())
3334
);
3435

35-
List<Messages.Envelope> envelopes = Gherkin.fromSources(
36+
List<Envelope> envelopes = Gherkin.fromSources(
3637
sources,
3738
true,
3839
true,
3940
true,
4041
() -> idGenerator.get().toString()
4142
).collect(toList());
4243

43-
GherkinDialect dialect = null;
44-
GherkinDocument gherkinDocument = null;
45-
List<Pickle> pickles = new ArrayList<>();
46-
for (Messages.Envelope envelope : envelopes) {
47-
if (envelope.hasGherkinDocument()) {
48-
gherkinDocument = envelope.getGherkinDocument();
49-
cucumberQuery.update(gherkinDocument);
50-
GherkinDialectProvider dialectProvider = new GherkinDialectProvider();
51-
String language = gherkinDocument.getFeature().getLanguage();
52-
dialect = dialectProvider.getDialect(language, null);
53-
}
54-
if (envelope.hasPickle()) {
55-
Messages.Pickle pickle = envelope.getPickle();
56-
pickles.add(new GherkinMessagesPickle(pickle, path, dialect, cucumberQuery));
57-
}
44+
GherkinDocument gherkinDocument = envelopes.stream()
45+
.filter(Envelope::hasGherkinDocument)
46+
.map(Envelope::getGherkinDocument)
47+
.findFirst()
48+
.orElse(null);
49+
50+
if (gherkinDocument == null || !gherkinDocument.hasFeature()) {
51+
return Optional.empty();
5852
}
5953

60-
return new GherkinMessagesFeature(gherkinDocument, path, source, pickles, envelopes);
54+
CucumberQuery cucumberQuery = new CucumberQuery();
55+
cucumberQuery.update(gherkinDocument);
56+
GherkinDialectProvider dialectProvider = new GherkinDialectProvider();
57+
String language = gherkinDocument.getFeature().getLanguage();
58+
GherkinDialect dialect = dialectProvider.getDialect(language, null);
59+
60+
List<Messages.Pickle> pickleMessages = envelopes.stream()
61+
.filter(Envelope::hasPickle)
62+
.map(Envelope::getPickle)
63+
.collect(toList());
64+
65+
if (pickleMessages.isEmpty()) {
66+
return Optional.empty();
67+
}
68+
69+
List<Pickle> pickles = pickleMessages.stream()
70+
.map(pickle -> new GherkinMessagesPickle(pickle, path, dialect, cucumberQuery))
71+
.collect(toList());
72+
73+
GherkinMessagesFeature feature = new GherkinMessagesFeature(
74+
gherkinDocument,
75+
path,
76+
source,
77+
pickles,
78+
envelopes
79+
);
80+
return Optional.of(feature);
6181
} catch (ParserException e) {
6282
throw new FeatureParserException("Failed to parse resource at: " + path.toString(), e);
6383
}

Diff for: ‎gherkin-messages/src/test/java/io/cucumber/core/gherkin/messages/FeatureParserTest.java

+35
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
1+
package io.cucumber.core.gherkin.messages;
2+
3+
import io.cucumber.core.gherkin.Feature;
4+
import org.junit.jupiter.api.Test;
5+
6+
import java.io.IOException;
7+
import java.net.URI;
8+
import java.nio.file.Paths;
9+
import java.util.Optional;
10+
import java.util.UUID;
11+
12+
import static java.nio.file.Files.readAllBytes;
13+
import static org.junit.jupiter.api.Assertions.assertFalse;
14+
15+
class FeatureParserTest {
16+
17+
private final GherkinMessagesFeatureParser parser = new GherkinMessagesFeatureParser();
18+
19+
@Test
20+
void feature_file_without_pickles_is_parsed_but_produces_no_feature() throws IOException {
21+
URI uri = URI.create("classpath:com/example.feature");
22+
String source = new String(readAllBytes(Paths.get("src/test/resources/io/cucumber/core/gherkin/messages/no-pickles.feature")));
23+
Optional<Feature> feature = parser.parse(uri, source, UUID::randomUUID);
24+
assertFalse(feature.isPresent());
25+
}
26+
27+
@Test
28+
void empty_feature_file_is_parsed_but_produces_no_feature() throws IOException {
29+
URI uri = URI.create("classpath:com/example.feature");
30+
String source = new String(readAllBytes(Paths.get("src/test/resources/io/cucumber/core/gherkin/messages/empty.feature")));
31+
Optional<Feature> feature = parser.parse(uri, source, UUID::randomUUID);
32+
assertFalse(feature.isPresent());
33+
}
34+
35+
}

Diff for: ‎gherkin-messages/src/test/resources/io/cucumber/core/gherkin/messages/empty.feature

Whitespace-only changes.

Diff for: ‎gherkin-messages/src/test/resources/io/cucumber/core/gherkin/messages/no-pickles.feature

+3
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
Feature: The first rule of the empty feature is no scenarios
2+
3+
Rule: Test

Diff for: ‎gherkin-vintage/src/main/java/io/cucumber/core/gherkin/vintage/GherkinVintageFeatureParser.java

+17-10
Original file line numberDiff line numberDiff line change
@@ -16,38 +16,45 @@
1616
import java.net.URI;
1717
import java.util.Collections;
1818
import java.util.List;
19+
import java.util.Optional;
1920
import java.util.UUID;
2021
import java.util.function.Supplier;
2122
import java.util.stream.Collectors;
2223

2324
public final class GherkinVintageFeatureParser implements FeatureParser {
24-
private static Feature parseGherkin5(URI path, String source) {
25+
private static Optional<Feature> parseGherkin5(URI path, String source) {
2526
try {
2627
Parser<GherkinDocument> parser = new Parser<>(new AstBuilder());
2728
TokenMatcher matcher = new TokenMatcher();
2829
GherkinDocument gherkinDocument = parser.parse(source, matcher);
29-
GherkinDialectProvider dialectProvider = new GherkinDialectProvider();
30-
List<Pickle> pickles = compilePickles(gherkinDocument, dialectProvider, path);
31-
return new GherkinVintageFeature(gherkinDocument, path, source, pickles);
30+
if(gherkinDocument.getFeature() == null){
31+
return Optional.empty();
32+
}
33+
List<Pickle> pickles = compilePickles(path, gherkinDocument);
34+
if (pickles.isEmpty()) {
35+
return Optional.empty();
36+
}
37+
GherkinVintageFeature feature = new GherkinVintageFeature(gherkinDocument, path, source, pickles);
38+
return Optional.of(feature);
3239
} catch (ParserException e) {
3340
throw new FeatureParserException("Failed to parse resource at: " + path.toString(), e);
3441
}
3542
}
3643

37-
private static List<Pickle> compilePickles(GherkinDocument document, GherkinDialectProvider dialectProvider, URI path) {
38-
if (document.getFeature() == null) {
39-
return Collections.emptyList();
40-
}
44+
private static List<Pickle> compilePickles(URI path, GherkinDocument document) {
45+
GherkinDialectProvider dialectProvider = new GherkinDialectProvider();
4146
String language = document.getFeature().getLanguage();
4247
GherkinDialect dialect = dialectProvider.getDialect(language, null);
4348
return new Compiler().compile(document)
4449
.stream()
45-
.map(pickle -> new GherkinVintagePickle(pickle, path, document, dialect))
50+
.map(pickle -> {
51+
return new GherkinVintagePickle(pickle, path, document, dialect);
52+
})
4653
.collect(Collectors.toList());
4754
}
4855

4956
@Override
50-
public Feature parse(URI path, String source, Supplier<UUID> idGenerator) {
57+
public Optional<Feature> parse(URI path, String source, Supplier<UUID> idGenerator) {
5158
return parseGherkin5(path, source);
5259
}
5360

Diff for: ‎gherkin-vintage/src/test/java/io/cucumber/core/gherkin/vintage/FeatureParserTest.java

+35
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
1+
package io.cucumber.core.gherkin.vintage;
2+
3+
import io.cucumber.core.gherkin.Feature;
4+
import org.junit.jupiter.api.Test;
5+
6+
import java.io.IOException;
7+
import java.net.URI;
8+
import java.nio.file.Paths;
9+
import java.util.Optional;
10+
import java.util.UUID;
11+
12+
import static java.nio.file.Files.readAllBytes;
13+
import static org.junit.jupiter.api.Assertions.assertFalse;
14+
15+
class FeatureParserTest {
16+
17+
@Test
18+
void empty_feature_file_is_parsed_but_produces_no_feature() throws IOException {
19+
GherkinVintageFeatureParser parser = new GherkinVintageFeatureParser();
20+
URI uri = URI.create("classpath:com/example.feature");
21+
String source = new String(readAllBytes(Paths.get("src/test/resources/io/cucumber/core/gherkin/vintage/empty.feature")));
22+
Optional<Feature> feature = parser.parse(uri, source, UUID::randomUUID);
23+
assertFalse(feature.isPresent());
24+
}
25+
26+
@Test
27+
void feature_file_without_pickles_is_parsed_but_produces_no_feature() throws IOException {
28+
GherkinVintageFeatureParser parser = new GherkinVintageFeatureParser();
29+
URI uri = URI.create("classpath:com/example.feature");
30+
String source = new String(readAllBytes(Paths.get("src/test/resources/io/cucumber/core/gherkin/vintage/empty.feature")));
31+
Optional<Feature> feature = parser.parse(uri, source, UUID::randomUUID);
32+
assertFalse(feature.isPresent());
33+
}
34+
35+
}

Diff for: ‎gherkin-vintage/src/test/resources/io/cucumber/core/gherkin/vintage/empty.feature

Whitespace-only changes.

Diff for: ‎gherkin-vintage/src/test/resources/io/cucumber/core/gherkin/vintage/no-pickles.feature

+1
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
Feature: No scenarios, no rules

Diff for: ‎gherkin/src/main/java/io/cucumber/core/gherkin/FeatureParser.java

+2-1
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,13 @@
11
package io.cucumber.core.gherkin;
22

33
import java.net.URI;
4+
import java.util.Optional;
45
import java.util.UUID;
56
import java.util.function.Supplier;
67

78
public interface FeatureParser {
89

9-
Feature parse(URI path, String source, Supplier<UUID> idGenerator);
10+
Optional<Feature> parse(URI path, String source, Supplier<UUID> idGenerator);
1011

1112
String version();
1213

Diff for: ‎java/src/test/java/io/cucumber/java/TestFeatureParser.java

+1-1
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,6 @@ public InputStream getInputStream() {
3232
return new ByteArrayInputStream(source.getBytes(StandardCharsets.UTF_8));
3333
}
3434

35-
});
35+
}).orElse(null);
3636
}
3737
}

Diff for: ‎java8/src/test/java/io/cucumber/java8/TestFeatureParser.java

+1-1
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,6 @@ public InputStream getInputStream() {
3232
return new ByteArrayInputStream(source.getBytes(StandardCharsets.UTF_8));
3333
}
3434

35-
});
35+
}).orElse(null);
3636
}
3737
}

0 commit comments

Comments
 (0)