Skip to content

Commit 7d5c96e

Browse files
tzezulatzezula
committed
[GR-14621] Fixed review comments.
1 parent 0434ba6 commit 7d5c96e

File tree

2 files changed

+52
-60
lines changed

2 files changed

+52
-60
lines changed

‎src/main/java/org/truffleruby/RubyFileTypeDetector.java

Lines changed: 45 additions & 51 deletions
Original file line numberDiff line numberDiff line change
@@ -18,89 +18,83 @@
1818
import java.util.regex.Pattern;
1919
import com.oracle.truffle.api.TruffleFile;
2020
import org.jcodings.Encoding;
21+
import org.jcodings.specific.UTF8Encoding;
2122
import org.truffleruby.core.encoding.EncodingManager;
2223
import org.truffleruby.core.rope.Rope;
2324
import org.truffleruby.core.string.StringOperations;
2425
import org.truffleruby.parser.lexer.RubyLexer;
26+
import org.truffleruby.shared.TruffleRuby;
2527

2628
public class RubyFileTypeDetector implements TruffleFile.FileTypeDetector {
2729

28-
private static final String MIME_TYPE = "application/x-ruby";
29-
30-
private static final String[] KNOWN_RUBY_FILES = new String[]{ "Gemfile", "Rakefile", "Mavenfile" };
30+
private static final String[] KNOWN_RUBY_FILES = new String[]{ "Gemfile", "Rakefile" };
3131
private static final String[] KNOWN_RUBY_SUFFIXES = new String[]{ ".rb", ".rake", ".gemspec" };
3232
private static final Pattern SHEBANG_REGEXP = Pattern.compile("^#! ?/usr/bin/(env +ruby|ruby).*");
3333

3434
@Override
3535
public String findMimeType(TruffleFile file) throws IOException {
36-
return findMimeAndEncodingImpl(file, null);
37-
}
38-
39-
@Override
40-
public Charset findEncoding(TruffleFile file) throws IOException {
41-
Charset[] encodingHolder = new Charset[1];
42-
findMimeAndEncodingImpl(file, encodingHolder);
43-
return encodingHolder[0];
44-
}
45-
46-
private String findMimeAndEncodingImpl(TruffleFile file, Charset[] encodingHolder) {
4736
final String fileName = file.getName();
4837

4938
if (fileName == null) {
5039
return null;
5140
}
5241

5342
final String lowerCaseFileName = fileName.toLowerCase(Locale.ROOT);
54-
String mimeType = null;
5543

5644
for (String candidate : KNOWN_RUBY_SUFFIXES) {
5745
if (lowerCaseFileName.endsWith(candidate)) {
58-
mimeType = MIME_TYPE;
59-
break;
46+
return TruffleRuby.MIME_TYPE;
6047
}
6148
}
6249

63-
if (mimeType == null) {
64-
for (String candidate : KNOWN_RUBY_FILES) {
65-
if (fileName.equals(candidate)) {
66-
mimeType = MIME_TYPE;
67-
break;
68-
}
50+
for (String candidate : KNOWN_RUBY_FILES) {
51+
if (fileName.equals(candidate)) {
52+
return TruffleRuby.MIME_TYPE;
6953
}
7054
}
7155

72-
if (mimeType == null || encodingHolder != null) {
73-
try (BufferedReader fileContent = file.newBufferedReader(StandardCharsets.UTF_8)) {
74-
final String firstLine = fileContent.readLine();
75-
if (firstLine != null) {
76-
String encodingCommentLine;
77-
if (SHEBANG_REGEXP.matcher(firstLine).matches()) {
78-
mimeType = mimeType == null ? MIME_TYPE : mimeType;
79-
encodingCommentLine = encodingHolder == null ? null : fileContent.readLine();
80-
} else {
81-
encodingCommentLine = encodingHolder == null ? null : firstLine;
82-
}
83-
if (encodingCommentLine != null) {
84-
Rope encodingCommentRope = StringOperations.encodeRope(encodingCommentLine, EncodingManager.getEncoding("UTF-8"));
85-
RubyLexer.parseMagicComment(encodingCommentRope, new BiConsumer<String, Rope>() {
86-
@Override
87-
public void accept(String name, Rope value) {
88-
if (RubyLexer.isMagicEncodingComment(name)) {
89-
Encoding encoding = EncodingManager.getEncoding(value);
90-
if (encoding != null) {
91-
encodingHolder[0] = encoding.getCharset();
92-
}
56+
try (BufferedReader fileContent = file.newBufferedReader(StandardCharsets.UTF_8)) {
57+
final String firstLine = fileContent.readLine();
58+
if (firstLine != null && SHEBANG_REGEXP.matcher(firstLine).matches()) {
59+
return TruffleRuby.MIME_TYPE;
60+
}
61+
} catch (IOException | SecurityException e) {
62+
// Reading random files as UTF-8 could cause all sorts of errors
63+
}
64+
return null;
65+
}
66+
67+
@Override
68+
public Charset findEncoding(TruffleFile file) throws IOException {
69+
try (BufferedReader fileContent = file.newBufferedReader(StandardCharsets.UTF_8)) {
70+
final String firstLine = fileContent.readLine();
71+
if (firstLine != null) {
72+
String encodingCommentLine;
73+
if (SHEBANG_REGEXP.matcher(firstLine).matches()) {
74+
encodingCommentLine = fileContent.readLine();
75+
} else {
76+
encodingCommentLine = firstLine;
77+
}
78+
if (encodingCommentLine != null) {
79+
Rope encodingCommentRope = StringOperations.encodeRope(encodingCommentLine, UTF8Encoding.INSTANCE);
80+
Charset[] encodingHolder = new Charset[1];
81+
RubyLexer.parseMagicComment(encodingCommentRope, new BiConsumer<String, Rope>() {
82+
@Override
83+
public void accept(String name, Rope value) {
84+
if (RubyLexer.isMagicEncodingComment(name)) {
85+
Encoding encoding = EncodingManager.getEncoding(value);
86+
if (encoding != null) {
87+
encodingHolder[0] = encoding.getCharset();
9388
}
9489
}
95-
});
96-
}
90+
}
91+
});
92+
return encodingHolder[0];
9793
}
98-
} catch (IOException | SecurityException e) {
99-
// Reading random files as UTF-8 could cause all sorts of errors
10094
}
95+
} catch (IOException | SecurityException e) {
96+
// Reading random files as UTF-8 could cause all sorts of errors
10197
}
102-
103-
return mimeType;
98+
return null;
10499
}
105-
106100
}

‎src/test/java/org/truffleruby/RubyFileTypeDetectorTest.java

Lines changed: 7 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -43,9 +43,9 @@ public void accept(RubyRootNode rootNode) {
4343
for (TestCase testCase : getTestCases()) {
4444
TruffleFile file = env.getTruffleFile(testCase.path.toString());
4545
if (testCase.hasRubyMimeType) {
46-
assertEquals(TruffleRuby.MIME_TYPE, fileTypeDetector.findMimeType(file));
46+
assertEquals(testCase.path.toString(), TruffleRuby.MIME_TYPE, fileTypeDetector.findMimeType(file));
4747
} else {
48-
assertNotEquals(TruffleRuby.MIME_TYPE, fileTypeDetector.findMimeType(file));
48+
assertNotEquals(testCase.path.toString(), TruffleRuby.MIME_TYPE, fileTypeDetector.findMimeType(file));
4949
}
5050
}
5151
} catch (IOException ioe) {
@@ -59,9 +59,9 @@ public void accept(RubyRootNode rootNode) {
5959
public void testIndirect() throws IOException {
6060
for (TestCase testCase : getTestCases()) {
6161
if (testCase.hasRubyMimeType) {
62-
assertEquals(TruffleRuby.MIME_TYPE, Source.findMimeType(testCase.path.toFile()));
62+
assertEquals(testCase.path.toString(), TruffleRuby.MIME_TYPE, Source.findMimeType(testCase.path.toFile()));
6363
} else {
64-
assertNotEquals(TruffleRuby.MIME_TYPE, Source.findMimeType(testCase.path.toFile()));
64+
assertNotEquals(testCase.path.toString(), TruffleRuby.MIME_TYPE, Source.findMimeType(testCase.path.toFile()));
6565
}
6666
}
6767
}
@@ -75,11 +75,9 @@ public void accept(RubyRootNode rootNode) {
7575
TruffleLanguage.Env env = rootNode.getContext().getEnv();
7676
try {
7777
for (TestCase testCase : getTestCases()) {
78-
TruffleFile file = env.getTruffleFile(testCase.path.toString());
7978
if (testCase.hasRubyMimeType) {
80-
assertEquals(TruffleRuby.MIME_TYPE, fileTypeDetector.findMimeType(file));
81-
} else {
82-
assertNotEquals(TruffleRuby.MIME_TYPE, fileTypeDetector.findMimeType(file));
79+
TruffleFile file = env.getTruffleFile(testCase.path.toString());
80+
assertEquals(testCase.encoding, fileTypeDetector.findEncoding(file));
8381
}
8482
}
8583
} catch (IOException ioe) {
@@ -99,7 +97,7 @@ private static TestCase[] getTestCases() throws IOException {
9997
testCases.add(new TestCase(createFile(tempDirectory, "TESTUP.RB", "puts 'hello'"), true, null));
10098
testCases.add(new TestCase(createFile(tempDirectory, "Gemfile", "puts 'hello'"), true, null));
10199
testCases.add(new TestCase(createFile(tempDirectory, "Rakefile", "puts 'hello'"), true, null));
102-
testCases.add(new TestCase(createFile(tempDirectory, "Mavenfile", "puts 'hello'"), true, null));
100+
testCases.add(new TestCase(createFile(tempDirectory, "Mavenfile", "puts 'hello'"), false, null));
103101
testCases.add(new TestCase(createFile(tempDirectory, "test.rake", "puts 'hello'"), true, null));
104102
testCases.add(new TestCase(createFile(tempDirectory, "test.gemspec", "puts 'hello'"), true, null));
105103
testCases.add(new TestCase(createFile(tempDirectory, "shebang", "#!/usr/bin/ruby\nputs 'hello'"), true, null));

0 commit comments

Comments
 (0)