From 48e5349bbec69d2da3d5d42cc732197f5f584701 Mon Sep 17 00:00:00 2001 From: Ruslan Cheremin Date: Fri, 25 Jul 2025 14:47:21 +0200 Subject: [PATCH] [vfs][refactoring] redefine `BatchingFileSystem` contract: always return case-sensitive maps ... thus it is responsibility of a caller to convert returned map to case-insensitive, if needed, and deal with possible inconsistencies during that conversion GitOrigin-RevId: 83b2c714a4e9cba62281d2bf2a05ebd04f643225 --- .../openapi/vfs/newvfs/ArchiveFileSystem.java | 5 ++-- .../newvfs/persistent/BatchingFileSystem.java | 18 +++++++++++---- .../vfs/impl/local/LocalFileSystemImpl.java | 3 ++- .../openapi/vfs/newvfs/RefreshWorker.java | 23 +++++++++++++++---- .../newvfs/persistent/PersistentFSImpl.java | 1 + 5 files changed, 37 insertions(+), 13 deletions(-) diff --git a/platform/analysis-api/src/com/intellij/openapi/vfs/newvfs/ArchiveFileSystem.java b/platform/analysis-api/src/com/intellij/openapi/vfs/newvfs/ArchiveFileSystem.java index eb1948f85554..84dfcfe763b2 100644 --- a/platform/analysis-api/src/com/intellij/openapi/vfs/newvfs/ArchiveFileSystem.java +++ b/platform/analysis-api/src/com/intellij/openapi/vfs/newvfs/ArchiveFileSystem.java @@ -165,10 +165,11 @@ public abstract class ArchiveFileSystem extends NewVirtualFileSystem implements ArchiveHandler handler = getHandler(dir); if (childNames == null) { - childNames = createFilePathSet(handler.list(directoryRelativePath), isCaseSensitive()); + childNames = createFilePathSet(handler.list(directoryRelativePath), /*isCaseSensitive: */ true); } - Map childrenWithAttributes = createFilePathMap(childNames.size(), isCaseSensitive()); + //We must return 'normal' (case-sensitive) map from this method, see BatchingFileSystem.listWithAttributes() contract: + Map childrenWithAttributes = createFilePathMap(childNames.size(), /*isCaseSensitive: */ true); for (String childName : childNames) { String childRelativePath = normalizedDirectoryPath + childName; diff --git a/platform/analysis-api/src/com/intellij/openapi/vfs/newvfs/persistent/BatchingFileSystem.java b/platform/analysis-api/src/com/intellij/openapi/vfs/newvfs/persistent/BatchingFileSystem.java index c63e0552d2ab..f18efaadfd02 100644 --- a/platform/analysis-api/src/com/intellij/openapi/vfs/newvfs/persistent/BatchingFileSystem.java +++ b/platform/analysis-api/src/com/intellij/openapi/vfs/newvfs/persistent/BatchingFileSystem.java @@ -18,12 +18,20 @@ import java.util.Set; @ApiStatus.Internal public interface BatchingFileSystem { /** - * Returns a list of files in a directory along with their attributes. - * When the {@code childrenNames} set is {@code null}, all files should be returned. - * TODO RC: specify, should returned map be case-(in)sensitive, according to dir.isCaseSensitive(), or it should be a plain - * case-sensitive map, and it is up to client code to transform it, if needed? + * @return directory's children, in form of Map[childName-> FileAttributes] + * When the {@code childrenNames} is not-null, only those children's data should be returned (if exists!), + * when {@code childrenNames} is null -> all directory's children should be returned. + * Returned map should be a 'normal' (i.e., case-sensitive) map -- it is up to calling code to covert it to case-insensitive, if + * needed. */ - @NotNull Map<@NotNull String, @NotNull FileAttributes> listWithAttributes(@NotNull VirtualFile dir, @Nullable Set childrenNames); + //Why contract defines returned map to be case-sensitive? Because dir.isCaseSensitive() is tricky, and sometimes it's value could + // be unreliable/outdated -- in which case this method would provide incorrect info, be it defined to return map with case-sensitivity + // =dir.isCaseSensitive(). + // But the responsibility of (Batching)FileSystem is _only_ to be golden-source of info about actual file system state -- it is + // the responsibility of the caller (=VFS) to deal with (possible) inconsistencies between VFS state and actual FS state. + //MAYBE RC: return List>? this way question of case-sensitivity is not even on the table + @NotNull Map<@NotNull String, @NotNull FileAttributes> listWithAttributes(@NotNull VirtualFile dir, + @Nullable Set childrenNames); default @NotNull Map<@NotNull String, @NotNull FileAttributes> listWithAttributes(@NotNull VirtualFile dir) { return listWithAttributes(dir, null); diff --git a/platform/platform-impl/src/com/intellij/openapi/vfs/impl/local/LocalFileSystemImpl.java b/platform/platform-impl/src/com/intellij/openapi/vfs/impl/local/LocalFileSystemImpl.java index d9dfc73c4731..fde38d7ae47b 100644 --- a/platform/platform-impl/src/com/intellij/openapi/vfs/impl/local/LocalFileSystemImpl.java +++ b/platform/platform-impl/src/com/intellij/openapi/vfs/impl/local/LocalFileSystemImpl.java @@ -372,7 +372,8 @@ public class LocalFileSystemImpl extends LocalFileSystemBase implements Disposab return Collections.emptyMap(); } try { - Map childrenWithAttributes = createFilePathMap(10, dir.isCaseSensitive()); + //We must return 'normal' (case-sensitive) map from this method, see BatchingFileSystem.listWithAttributes() contract: + Map childrenWithAttributes = createFilePathMap(10, /*caseSensitive: */true); PlatformNioHelper.visitDirectory(Path.of(toIoPath(dir)), filter, (file, ioAttributesHolder) -> { try { diff --git a/platform/platform-impl/src/com/intellij/openapi/vfs/newvfs/RefreshWorker.java b/platform/platform-impl/src/com/intellij/openapi/vfs/newvfs/RefreshWorker.java index fd2c36203832..d2b863774593 100644 --- a/platform/platform-impl/src/com/intellij/openapi/vfs/newvfs/RefreshWorker.java +++ b/platform/platform-impl/src/com/intellij/openapi/vfs/newvfs/RefreshWorker.java @@ -255,6 +255,11 @@ final class RefreshWorker { for (String name : childrenNames) { childrenWithAttributes.put(name, null); } + if(childrenWithAttributes.size()!=childrenNames.length){ + //TODO RC: seems like dir.isCaseSensitive() is wrong/outdated (i.e. actual dir case-sensitivity is different from + // FS-default, and it wasn't yet determined). + // We should re-query dir.case-sensitivity + } } myIoTime.addAndGet(System.nanoTime() - t); @@ -401,15 +406,23 @@ final class RefreshWorker { } /** Converts a case-sensitive rawDirList map into case-insensitive, if toCaseSensitive=false, leaves the map as-is otherwise */ - private static @NotNull Map adjustCaseSensitivity(@NotNull Map rawDirList, + private static @NotNull Map adjustCaseSensitivity(@NotNull Map childrenWithAttributes, boolean toCaseSensitive) { if (toCaseSensitive) { - return rawDirList; + return childrenWithAttributes; } else { - Map filtered = createFilePathMap(rawDirList.size(), /*caseSensitive: */ false); - filtered.putAll(rawDirList); - return filtered; + Map childrenWithAttributesCaseInsensitive = createFilePathMap( + childrenWithAttributes.size(), + /*caseSensitive: */ false + ); + childrenWithAttributesCaseInsensitive.putAll(childrenWithAttributes); + if (childrenWithAttributesCaseInsensitive.size() != childrenWithAttributes.size()) { + //TODO RC: seems like a conflict if dir.isCaseSensitive() is wrong/outdated (i.e. actual dir case-sensitivity + // is different from FS-default, and it wasn't yet determined). + // We should re-query dir.case-sensitivity + } + return childrenWithAttributesCaseInsensitive; } } diff --git a/platform/platform-impl/src/com/intellij/openapi/vfs/newvfs/persistent/PersistentFSImpl.java b/platform/platform-impl/src/com/intellij/openapi/vfs/newvfs/persistent/PersistentFSImpl.java index 0800c3d8bfe1..42d3fb517439 100644 --- a/platform/platform-impl/src/com/intellij/openapi/vfs/newvfs/persistent/PersistentFSImpl.java +++ b/platform/platform-impl/src/com/intellij/openapi/vfs/newvfs/persistent/PersistentFSImpl.java @@ -456,6 +456,7 @@ public final class PersistentFSImpl extends PersistentFS implements Disposable { //TODO RC: we use a map here to prevent duplicates -- but we still add those duplicates to childrenToAdd // -- what's the point? + //MAYBE RC: duplicates may indicate wrongly-detected dir.caseSensitivity -- so we should consider re-detect it? ChildInfo newChild = justCreated.computeIfAbsent( newChildName, _newChildName -> makeChildRecord(dir, dirId, _newChildName, childData, fs, null)