clarify FileTypeIdentifiableByVirtualFile.isMyType() method as not exhaustive "file belongs to this file type" but only as one of the possible checks, because otherwise too much code would need to be duplicated

GitOrigin-RevId: 3277a316a60082bd17f7ea7e263aed278ab93947
This commit is contained in:
Alexey Kudravtsev
2021-11-22 15:46:30 +00:00
committed by intellij-monorepo-bot
parent e6355de21a
commit 4c0cb063e5
4 changed files with 33 additions and 19 deletions
@@ -30,9 +30,9 @@ import org.jetbrains.annotations.NotNull;
*/
public interface FileTypeIdentifiableByVirtualFile extends FileType {
/**
* @return true if (and only if) this particular file should be treated as belonging to this file type.
* Please make sure your definition is consistent with all other definitions for this file type.
* For example, file type should not be associated with the file name pattern (see Settings|Editor|File Types) which is not mentioned in the {@code isMyFileType()} below.
* @return true if this particular file should be treated as belonging to this file type.
* Note that this file type can be associated with other files by other means as well (e.g., "Settings|Editor|File Types|Associate file name pattern..."),
* so this method is just one of the possible file type definitions.
*/
boolean isMyFileType(@NotNull VirtualFile file);
@@ -668,10 +668,11 @@ public class FileTypeManagerImpl extends FileTypeManagerEx implements Persistent
}
}
if (requestedFileType instanceof FileTypeIdentifiableByVirtualFile) {
return ((FileTypeIdentifiableByVirtualFile)requestedFileType).isMyFileType(file);
if (requestedFileType instanceof FileTypeIdentifiableByVirtualFile
&& ((FileTypeIdentifiableByVirtualFile)requestedFileType).isMyFileType(file)) {
return true;
}
// otherwise we can skip all the mySpecialFileTypes because it's certain this file type is not one of them
// otherwise, we can skip all the mySpecialFileTypes because it's certain this file type is not one of them
FileType fileType = getFileTypeByFileName(file.getNameSequence());
if (fileType == UnknownFileType.INSTANCE) {
@@ -1408,15 +1408,26 @@ public class FileTypesTest extends HeavyPlatformTestCase {
public void testIsFileOfTypeMustNotQueryAllFileTypesIdentifiableByVirtualFileForPerformanceReasons() throws IOException {
Disposable disposable = Disposer.newDisposable();
try {
AtomicInteger count = new AtomicInteger();
AtomicInteger myFileTypeCalledCount = new AtomicInteger();
class MyFileTypeIdentifiableByFile extends FakeFileType {
@Override public boolean isMyFileType(@NotNull VirtualFile file) { count.incrementAndGet(); return false; }
@Override public boolean isMyFileType(@NotNull VirtualFile file) { myFileTypeCalledCount.incrementAndGet(); return false; }
@Override public @NotNull String getName() { return "myfake"; }
@Override public @Nls @NotNull String getDisplayName() { return getName(); }
@Override public @NotNull @NlsContexts.Label String getDescription() { return getName(); }
}
FileType myType = new MyFileTypeIdentifiableByFile();
myFileTypeManager.registerFileType(myType, List.of(), disposable);
myFileTypeManager.registerFileType(new MyFileTypeIdentifiableByFile(), List.of(), disposable);
AtomicInteger otherFileTypeCalledCount = new AtomicInteger();
class MyOtherFileTypeIdentifiableByFile extends FakeFileType {
@Override public boolean isMyFileType(@NotNull VirtualFile file) {
otherFileTypeCalledCount.incrementAndGet();
return false;
}
@Override public @NotNull String getName() { return "myotherfake"; }
@Override public @Nls @NotNull String getDisplayName() { return getName(); }
@Override public @NotNull @NlsContexts.Label String getDescription() { return getName(); }
}
myFileTypeManager.registerFileType(new MyOtherFileTypeIdentifiableByFile(), List.of(), disposable);
File f = createTempFile("xx.lkj_lkj_lkj_ljk", "a");
VirtualFile virtualFile = getVirtualFile(f);
@@ -1424,7 +1435,7 @@ public class FileTypesTest extends HeavyPlatformTestCase {
FakeVirtualFile vf = new FakeVirtualFile(virtualFile, "myname.myname") {
@Override
public @NotNull FileType getFileType() {
return myFileTypeManager.getFileTypeByFile(this); // otherwise this call will be redirected to FileTypeManger.getIsntance() which is not what we are testing
return myFileTypeManager.getFileTypeByFile(this); // otherwise this call will be redirected to FileTypeManger.getInstance() which is not what we are testing
}
@Override
@@ -1434,17 +1445,19 @@ public class FileTypesTest extends HeavyPlatformTestCase {
};
FileType ft = myFileTypeManager.getFileTypeByFile(vf);
assertEquals(UnknownFileType.INSTANCE, ft);
assertTrue(count.toString(), count.get() > 0);
// during getFileType() we must check all possible file types
assertTrue(myFileTypeCalledCount.toString(), myFileTypeCalledCount.get() > 0);
assertTrue(otherFileTypeCalledCount.toString(), otherFileTypeCalledCount.get() > 0);
count.set(0);
myFileTypeCalledCount.set(0);
otherFileTypeCalledCount.set(0);
assertFalse(myFileTypeManager.isFileOfType(vf, PlainTextFileType.INSTANCE));
assertEquals(count.toString(), 0, count.get());
assertEquals(myFileTypeCalledCount.toString(), 0, myFileTypeCalledCount.get());
assertEquals(otherFileTypeCalledCount.toString(), 0, otherFileTypeCalledCount.get());
class MyOtherFileTypeIdentifiableByFile extends MyFileTypeIdentifiableByFile {
@Override public boolean isMyFileType(@NotNull VirtualFile file) { return false; }
}
assertFalse(myFileTypeManager.isFileOfType(vf, new MyOtherFileTypeIdentifiableByFile()));
assertEquals(count.toString(), 0, count.get());
assertEquals(myFileTypeCalledCount.toString(), 0, myFileTypeCalledCount.get()); // must not call irrelevant file types
assertTrue(otherFileTypeCalledCount.toString(), otherFileTypeCalledCount.get() > 0); // must call requested file type
}
finally {
Disposer.dispose(disposable);
@@ -28,7 +28,7 @@ object TomlFileType : LanguageFileType(TomlLanguage), FileTypeIdentifiableByVirt
override fun getCharset(file: VirtualFile, content: ByteArray): String = "UTF-8"
override fun isMyFileType(file: VirtualFile): Boolean {
return file.name == "config" && file.parent?.name == ".cargo" || FileTypeManager.getInstance().getFileTypeByFileName(file.name) == this
return file.name == "config" && file.parent?.name == ".cargo"
}
}