Skip to content

Commit

Permalink
#527 - IgnoreFilesIndex refactoring
Browse files Browse the repository at this point in the history
  • Loading branch information
hsz committed Apr 10, 2018
1 parent 8a4797a commit 24d0b06
Show file tree
Hide file tree
Showing 7 changed files with 66 additions and 52 deletions.
12 changes: 7 additions & 5 deletions src/mobi/hsz/idea/gitignore/IgnoreManager.java
Expand Up @@ -73,7 +73,7 @@
import java.util.List;
import java.util.Map;
import java.util.concurrent.ConcurrentMap;
import java.util.regex.Matcher;
import java.util.regex.Pattern;

import static mobi.hsz.idea.gitignore.IgnoreManager.RefreshTrackedIgnoredListener.TRACKED_IGNORED_REFRESH;
import static mobi.hsz.idea.gitignore.IgnoreManager.TrackedIgnoredListener.TRACKED_IGNORED;
Expand Down Expand Up @@ -404,10 +404,12 @@ public boolean isFileIgnored(@NotNull final VirtualFile file) {
relativePath += "/";
}

for (Pair<Matcher, Boolean> item : value.getItems()) {
if (matcher.match(item.first, relativePath)) {
ignored = !item.second;
matched = true;
if (value.getItems() != null) {
for (Pair<Pattern, Boolean> item : value.getItems()) {
if (matcher.match(item.first, relativePath)) {
ignored = !item.second;
matched = true;
}
}
}
}
Expand Down
Expand Up @@ -46,7 +46,6 @@

import java.util.Collection;
import java.util.List;
import java.util.regex.Matcher;
import java.util.regex.Pattern;

/**
Expand Down Expand Up @@ -112,7 +111,6 @@ private boolean isEntryExcluded(@NotNull IgnoreEntry entry, @NotNull Project pro
return false;
}

final Matcher matcher = pattern.matcher("");
final VirtualFile projectRoot = project.getBaseDir();
final List<VirtualFile> matched = ContainerUtil.newArrayList();
final Collection<VirtualFile> files = cache.getFilesForPattern(project, pattern);
Expand All @@ -128,7 +126,7 @@ private boolean isEntryExcluded(@NotNull IgnoreEntry entry, @NotNull Project pro
continue;
}
String path = Utils.getRelativePath(projectRoot, root);
if (manager.getMatcher().match(matcher, path)) {
if (manager.getMatcher().match(pattern, path)) {
matched.add(file);
return false;
}
Expand Down
57 changes: 34 additions & 23 deletions src/mobi/hsz/idea/gitignore/indexing/IgnoreEntryOccurrence.java
Expand Up @@ -27,7 +27,6 @@
import com.intellij.openapi.util.Pair;
import com.intellij.openapi.vfs.VirtualFile;
import com.intellij.openapi.vfs.VirtualFileManager;
import com.intellij.util.containers.ContainerUtil;
import org.apache.commons.lang.StringUtils;
import org.apache.commons.lang.builder.HashCodeBuilder;
import org.jetbrains.annotations.NotNull;
Expand All @@ -37,8 +36,6 @@
import java.io.DataOutput;
import java.io.IOException;
import java.io.Serializable;
import java.util.List;
import java.util.regex.Matcher;
import java.util.regex.Pattern;

/**
Expand All @@ -58,8 +55,8 @@ public class IgnoreEntryOccurrence implements Serializable {
private VirtualFile file;

/** Collection of ignore entries converted to {@link Pattern}. */
@NotNull
private final List<Pair<Matcher, Boolean>> items = ContainerUtil.newArrayList();
@Nullable
private Pair<Pattern, Boolean>[] items;

This comment has been minimized.

Copy link
@nicity

nicity Apr 11, 2018

items is potentially mutable variable


/**
* Constructor.
Expand Down Expand Up @@ -97,7 +94,15 @@ public IgnoreEntryOccurrence(@NotNull String url, @Nullable VirtualFile file) {
*/
@Override
public int hashCode() {
return new HashCodeBuilder().append(url).append(items.toString()).toHashCode();
HashCodeBuilder builder = new HashCodeBuilder().append(url);

if (items != null) {
for (Pair<Pattern, Boolean> item : items) {
builder = builder.append(item.first).append(item.second);
}
}

return builder.toHashCode();
}

/**
Expand All @@ -113,9 +118,16 @@ public boolean equals(@Nullable Object obj) {
}

final IgnoreEntryOccurrence entry = (IgnoreEntryOccurrence) obj;
boolean equals = url.equals(entry.url) && items.size() == entry.items.size();
for (int i = 0; i < items.size(); i++) {
equals = equals && items.get(i).toString().equals(entry.items.get(i).toString());
if (items == null && entry.items == null) {
return true;
}
if (items == null || entry.items == null) {
return false;
}

boolean equals = url.equals(entry.url) && items.length == entry.items.length;
for (int i = 0; i < items.length; i++) {
equals = equals && items[i].toString().equals(entry.items[i].toString());

This comment has been minimized.

Copy link
@nicity

nicity Apr 11, 2018

equals = equals is always true

This comment has been minimized.

Copy link
@fractile81

fractile81 Apr 11, 2018

This could be simplified without equals being a variable.

if (!url.equals(entry.url) || items.length != entry.items.length) return false;

for (int i = 0; i < items.length; i++) {
    if (!items[i].toString().equals(entry.items[i].toString())) return false
}

return true;
}

return equals;
Expand All @@ -139,19 +151,14 @@ public VirtualFile getFile() {
*
* @return entries
*/
@NotNull
public List<Pair<Matcher, Boolean>> getItems() {
@Nullable
public Pair<Pattern, Boolean>[] getItems() {
return items;
}

/**
* Adds new element to {@link #items}.
*
* @param matcher entry converted to {@link Matcher}
* @param isNegated entry is negated
*/
public void add(@NotNull Matcher matcher, boolean isNegated) {
items.add(Pair.create(matcher, isNegated));
/** Set entries for current file. */
public void setItems(@Nullable Pair<Pattern, Boolean>[] items) {

This comment has been minimized.

Copy link
@nicity

nicity Apr 11, 2018

immutable classes are better to have final fields and no set methods

this.items = items;
}

/**
Expand All @@ -164,9 +171,9 @@ public void add(@NotNull Matcher matcher, boolean isNegated) {
public static synchronized void serialize(@NotNull DataOutput out, @NotNull IgnoreEntryOccurrence entry)
throws IOException {
out.writeUTF(entry.url);
out.writeInt(entry.items.size());
for (Pair<Matcher, Boolean> item : entry.items) {
out.writeUTF(item.first.pattern().pattern());
out.writeInt(entry.items == null ? 0 : entry.items.length);
for (Pair<Pattern, Boolean> item : entry.items) {
out.writeUTF(item.first.pattern());
out.writeBoolean(item.second);
}
}
Expand All @@ -186,11 +193,15 @@ public static synchronized IgnoreEntryOccurrence deserialize(@NotNull DataInput

final IgnoreEntryOccurrence entry = new IgnoreEntryOccurrence(url);
int size = in.readInt();

@SuppressWarnings("unchecked")
Pair<Pattern, Boolean>[] items = (Pair<Pattern, Boolean>[]) new Pair[size];
for (int i = 0; i < size; i++) {
final Pattern pattern = Pattern.compile(in.readUTF());
Boolean isNegated = in.readBoolean();
entry.add(pattern.matcher(""), isNegated);
items[i] = Pair.create(pattern, isNegated);
}
entry.setItems(items);

return entry;
}
Expand Down
9 changes: 8 additions & 1 deletion src/mobi/hsz/idea/gitignore/indexing/IgnoreFilesIndex.java
Expand Up @@ -26,6 +26,7 @@

import com.intellij.openapi.application.ApplicationManager;
import com.intellij.openapi.project.Project;
import com.intellij.openapi.util.Pair;
import com.intellij.openapi.vfs.VirtualFile;
import com.intellij.psi.search.GlobalSearchScope;
import com.intellij.util.Function;
Expand Down Expand Up @@ -94,16 +95,22 @@ public Map<IgnoreFileTypeKey, IgnoreEntryOccurrence> map(@NotNull final FileCont
final IgnoreFileType type = (IgnoreFileType) inputData.getFileType();

IgnoreFile ignoreFile = (IgnoreFile) inputData.getPsiFile();
final List<Pair<Pattern, Boolean>> items = ContainerUtil.newArrayList();

ignoreFile.acceptChildren(new IgnoreVisitor() {
@Override
public void visitEntry(@NotNull IgnoreEntry entry) {
final Pattern pattern = Glob.createPattern(entry);
if (pattern != null) {
result.add(pattern.matcher(""), entry.isNegated());
items.add(Pair.create(pattern, entry.isNegated()));
}
}
});

@SuppressWarnings("unchecked")
Pair<Pattern, Boolean>[] arr = (Pair<Pattern, Boolean>[]) new Pair[0];
result.setItems(items.toArray(arr));

return Collections.singletonMap(new IgnoreFileTypeKey(type), result);
}

Expand Down
Expand Up @@ -48,7 +48,6 @@
import java.util.Collections;
import java.util.List;
import java.util.concurrent.ConcurrentMap;
import java.util.regex.Matcher;
import java.util.regex.Pattern;

/**
Expand Down Expand Up @@ -234,7 +233,6 @@ protected void innerResolveInContext(@NotNull String text, @NotNull PsiFileSyste
final VirtualFile root = isOuterFile ? contextVirtualFile : ((parent != null) ?
parent.getVirtualFile() : null);
final PsiManager psiManager = getElement().getManager();
final Matcher matcher = pattern.matcher("");

final List<VirtualFile> files = ContainerUtil.createLockFreeCopyOnWriteList();
files.addAll(filesIndexCache.getFilesForPattern(context.getProject(), pattern));
Expand Down Expand Up @@ -276,7 +274,7 @@ public boolean visitFile(@NotNull VirtualFile file) {
}

final String name = (root != null) ? Utils.getRelativePath(root, file) : file.getName();
if (manager.getMatcher().match(matcher, name)) {
if (manager.getMatcher().match(pattern, name)) {
PsiFileSystemItem psiFileSystemItem = getPsiFileSystemItem(psiManager, file);
if (psiFileSystemItem == null) {
continue;
Expand Down
18 changes: 8 additions & 10 deletions src/mobi/hsz/idea/gitignore/util/Glob.java
Expand Up @@ -39,7 +39,6 @@
import java.util.Map;
import java.util.Set;
import java.util.concurrent.ConcurrentMap;
import java.util.regex.Matcher;
import java.util.regex.Pattern;
import java.util.regex.PatternSyntaxException;

Expand Down Expand Up @@ -87,23 +86,22 @@ public static Map<IgnoreEntry, List<VirtualFile>> find(@NotNull final VirtualFil
@NotNull final MatcherUtil matcher,
final boolean includeNested) {
final ConcurrentMap<IgnoreEntry, List<VirtualFile>> result = ContainerUtil.newConcurrentMap();
final HashMap<IgnoreEntry, Matcher> map = ContainerUtil.newHashMap();
final HashMap<IgnoreEntry, Pattern> map = ContainerUtil.newHashMap();

for (IgnoreEntry entry : entries) {
result.put(entry, ContainerUtil.<VirtualFile>newArrayList());

final Pattern pattern = createPattern(entry);
if (pattern == null) {
continue;
if (pattern != null) {
map.put(entry, pattern);
}
map.put(entry, pattern.matcher(""));
}

VirtualFileVisitor<HashMap<IgnoreEntry, Matcher>> visitor =
new VirtualFileVisitor<HashMap<IgnoreEntry, Matcher>>(VirtualFileVisitor.NO_FOLLOW_SYMLINKS) {
VirtualFileVisitor<HashMap<IgnoreEntry, Pattern>> visitor =
new VirtualFileVisitor<HashMap<IgnoreEntry, Pattern>>(VirtualFileVisitor.NO_FOLLOW_SYMLINKS) {
@Override
public boolean visitFile(@NotNull VirtualFile file) {
final HashMap<IgnoreEntry, Matcher> current = ContainerUtil.newHashMap(getCurrentValue());
final HashMap<IgnoreEntry, Pattern> current = ContainerUtil.newHashMap(getCurrentValue());
if (current.isEmpty()) {
return false;
}
Expand All @@ -113,8 +111,8 @@ public boolean visitFile(@NotNull VirtualFile file) {
return false;
}

for (Map.Entry<IgnoreEntry, Matcher> item : current.entrySet()) {
final Matcher value = item.getValue();
for (Map.Entry<IgnoreEntry, Pattern> item : current.entrySet()) {
final Pattern value = item.getValue();
boolean matches = false;
if (value == null || matcher.match(value, path)) {
matches = true;
Expand Down
14 changes: 7 additions & 7 deletions src/mobi/hsz/idea/gitignore/util/MatcherUtil.java
Expand Up @@ -48,25 +48,25 @@ public class MatcherUtil {
* Extracts alphanumeric parts from the regex pattern and checks if any of them is contained in the tested path.
* Looking for the parts speed ups the matching and prevents from running whole regex on the string.
*
* @param matcher to explode
* @param pattern to explode
* @param path to check
* @return path matches the pattern
*/
public boolean match(@Nullable Matcher matcher, @Nullable String path) {
if (matcher == null || path == null) {
public boolean match(@Nullable Pattern pattern, @Nullable String path) {
if (pattern == null || path == null) {
return false;
}

synchronized (cache) {
int hashCode = new HashCodeBuilder().append(matcher.pattern()).append(path).toHashCode();
int hashCode = new HashCodeBuilder().append(pattern).append(path).toHashCode();

if (!cache.containsKey(hashCode)) {
final String[] parts = getParts(matcher);
final String[] parts = getParts(pattern);
boolean result = false;

if (parts.length == 0 || matchAllParts(parts, path)) {
try {
result = matcher.reset(path).find();
result = pattern.matcher(path).find();
} catch (StringIndexOutOfBoundsException ignored) {
}
}
Expand Down Expand Up @@ -166,6 +166,6 @@ public static String[] getParts(@Nullable Pattern pattern) {
inSquare = ch != ']' && ((ch == '[') || inSquare);
}

return parts.toArray(new String[parts.size()]);
return parts.toArray(new String[0]);
}
}

0 comments on commit 24d0b06

Please sign in to comment.