Skip to content

Commit

Permalink
only filters labels according to parent packages for LanguageClass.C (#…
Browse files Browse the repository at this point in the history
…47)

* only filters labels according to parent packages for LanguageClass.C
This is to keep supporting CPP need while also supporting aggregate targets
More info:
https://bazelbuild.slack.com/archives/CM8JQCANN/p1566481600003600
Filter was deliberately added to fix an issue with C++ headers.
The original bug was that opening the BUILD file for foo/bar.h caused it to open baz/qux/BUILD because there’s a cc_library target in baz/qux that includes foo/bar.h in its headers. The source-to-target look up queries the index for the target during the last sync which compiled the source file. In this case, it was the cc_library target in baz/qux.
So the fix was to ensure that the BUILD file is always for the parent package (foo/bar)
  • Loading branch information
ittaiz committed Nov 14, 2019
1 parent d1bda8d commit aa9895d
Showing 1 changed file with 29 additions and 2 deletions.
31 changes: 29 additions & 2 deletions base/src/com/google/idea/blaze/base/actions/BuildFileUtils.java
Original file line number Diff line number Diff line change
Expand Up @@ -25,18 +25,21 @@
import com.google.idea.blaze.base.lang.buildfile.search.BlazePackage;
import com.google.idea.blaze.base.model.BlazeProjectData;
import com.google.idea.blaze.base.model.primitives.Label;
import com.google.idea.blaze.base.model.primitives.LanguageClass;
import com.google.idea.blaze.base.model.primitives.WorkspacePath;
import com.google.idea.blaze.base.sync.data.BlazeProjectDataManager;
import com.google.idea.blaze.base.targetmaps.SourceToTargetMap;
import com.google.idea.common.experiments.BoolExperiment;
import com.intellij.openapi.project.Project;
import com.intellij.openapi.util.io.FileUtilRt;
import com.intellij.openapi.vfs.VirtualFile;
import com.intellij.psi.PsiElement;
import com.intellij.psi.PsiFileSystemItem;
import com.intellij.psi.PsiManager;
import java.io.File;
import java.util.Arrays;
import java.util.Set;
import java.util.stream.Stream;
import javax.annotation.Nullable;

/** BUILD-file utility methods used by actions. */
Expand Down Expand Up @@ -74,9 +77,11 @@ static PsiElement findBuildTarget(Project project, BlazePackage parentPackage, F
if (packagePath == null) {
return null;
}
final Stream<Label> targetLabels = SourceToTargetMap.getInstance(project)
.getTargetsToBuildForSourceFile(file).stream();

Label label =
SourceToTargetMap.getInstance(project).getTargetsToBuildForSourceFile(file).stream()
.filter(l -> l.blazePackage().equals(packagePath))
filterCRelatedLabelsWhichBelongToOtherPackages(file,packagePath, targetLabels)
.findFirst()
.orElse(null);
if (label == null) {
Expand All @@ -98,6 +103,28 @@ static PsiElement findBuildTarget(Project project, BlazePackage parentPackage, F
return null;
}

private static Stream<Label> filterCRelatedLabelsWhichBelongToOtherPackages(File file,
WorkspacePath packagePath, Stream<Label> targetLabels) {
/*
More info:
https://bazelbuild.slack.com/archives/CM8JQCANN/p1566481600003600
Filter was deliberately added to fix an issue with C++ headers.
The original bug was that opening the BUILD file for foo/bar.h caused it to open baz/qux/BUILD
because there’s a cc_library target in baz/qux that includes foo/bar.h in its headers.
The source-to-target look up queries the index for the target during the last sync which compiled the source file.
In this case, it was the cc_library target in baz/qux.
So the fix was to ensure that the BUILD file is always for the parent package (foo/bar).
This is limited to C only since this logic messes up supporting aggregate targets
https://github.com/bazelbuild/intellij/issues/475
*/
LanguageClass fileLanguage = LanguageClass.fromExtension(
FileUtilRt.getExtension(file.getName()).toLowerCase());
return (fileLanguage == LanguageClass.C) ?
targetLabels.filter(l -> l.blazePackage().equals(packagePath)) :
targetLabels;
}

/**
* Returns the label of a macro with name prefixing the target name of a given label with a '_' or
* '-' delimiter.
Expand Down

0 comments on commit aa9895d

Please sign in to comment.