Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Hide EditorBrowsableNever members by default #4618

Merged
merged 6 commits into from
Nov 8, 2022
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 10 additions & 1 deletion src/dotnet/APIView/APIView/CodeFileRenderer.cs
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ private void Render(List<CodeLine> list, IEnumerable<CodeFileToken> node, bool s
var stringBuilder = new StringBuilder();
string currentId = null;
bool isDocumentationRange = false;
bool isHiddenApiRange = false;
bool isDeprecatedToken = false;
bool isSkipDiffRange = false;
Stack<SectionType> nodesInProcess = new Stack<SectionType>();
Expand All @@ -56,7 +57,7 @@ private void Render(List<CodeLine> list, IEnumerable<CodeFileToken> node, bool s
{
case CodeFileTokenKind.Newline:
int ? sectionKey = (nodesInProcess.Count > 0 && section == null) ? sections.Count: null;
CodeLine codeLine = new CodeLine(stringBuilder.ToString(), currentId, String.Empty, ++lineNumber, sectionKey, isDocumentation: isDocumentationRange);
CodeLine codeLine = new CodeLine(stringBuilder.ToString(), currentId, String.Empty, ++lineNumber, sectionKey, isDocumentation: isDocumentationRange, isHiddenApi: isHiddenApiRange);
if (leafSectionPlaceHolderNumber != 0)
{
lineNumber += leafSectionPlaceHolderNumber - 1;
Expand Down Expand Up @@ -104,6 +105,14 @@ private void Render(List<CodeLine> list, IEnumerable<CodeFileToken> node, bool s
isDocumentationRange = false;
break;

case CodeFileTokenKind.HiddenApiRangeStart:
isHiddenApiRange = true;
break;

case CodeFileTokenKind.HiddenApiRangeEnd:
isHiddenApiRange = false;
break;

case CodeFileTokenKind.DeprecatedRangeStart:
isDeprecatedToken = true;
break;
Expand Down
9 changes: 6 additions & 3 deletions src/dotnet/APIView/APIView/CodeLine.cs
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,9 @@ namespace ApiView
public int Indent { get; }
public bool IsDocumentation { get; }
public TreeNode<CodeLine> NodeRef { get; }
public bool IsHiddenApi { get; }

public CodeLine(string html, string id, string lineClass, int? lineNumber = null, int? sectionKey = null, int indent = 0, bool isDocumentation = false, TreeNode<CodeLine> nodeRef = null)
public CodeLine(string html, string id, string lineClass, int? lineNumber = null, int? sectionKey = null, int indent = 0, bool isDocumentation = false, TreeNode<CodeLine> nodeRef = null, bool isHiddenApi = false)
{
this.DisplayString = html;
this.ElementId = id;
Expand All @@ -24,9 +25,10 @@ public CodeLine(string html, string id, string lineClass, int? lineNumber = null
this.Indent = indent;
this.IsDocumentation = isDocumentation;
this.NodeRef = nodeRef;
this.IsHiddenApi = isHiddenApi;
}

public CodeLine(CodeLine codeLine, string html = null, string id = null, string lineClass = null, int? lineNumber = null, int? sectionKey = null, int indent = 0, bool isDocumentation = false, TreeNode<CodeLine> nodeRef = null)
public CodeLine(CodeLine codeLine, string html = null, string id = null, string lineClass = null, int? lineNumber = null, int? sectionKey = null, int indent = 0, bool isDocumentation = false, TreeNode<CodeLine> nodeRef = null, bool isHiddenApi = false)
{
this.DisplayString = html ?? codeLine.DisplayString;
this.ElementId = id ?? codeLine.ElementId;
Expand All @@ -36,6 +38,7 @@ public CodeLine(CodeLine codeLine, string html = null, string id = null, string
this.Indent = (indent != 0)? indent : codeLine.Indent;
this.IsDocumentation = (isDocumentation != false)? isDocumentation : codeLine.IsDocumentation;
this.NodeRef = nodeRef ?? codeLine.NodeRef;
this.IsHiddenApi = isHiddenApi;
}

public override string ToString()
Expand All @@ -45,7 +48,7 @@ public override string ToString()

public bool Equals(CodeLine other)
{
return DisplayString == other.DisplayString && ElementId == other.ElementId;
return DisplayString == other.DisplayString && ElementId == other.ElementId && other.IsHiddenApi == IsHiddenApi;
Copy link
Member Author

@JoshLove-msft JoshLove-msft Nov 4, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My assumption is that this is safe because all out-dated codefiles will be updated in the background to the latest version. If we think this is too dangerous I can make the new property nullable and add a safeguard such that if either is null, we don't consider it a difference. This way old files containing hidden APIs will not trigger a diff against new files. We could also leave the values as null for non C# languages so that they would not run into the same issue if ever enabling support for hidden API hiding.

However, it would be a lot simpler if we could rely on the background update process.

}
}
}
68 changes: 57 additions & 11 deletions src/dotnet/APIView/APIView/Languages/CodeFileBuilder.cs
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
using System;
using System.Collections.Generic;
using System.Collections.Immutable;
using System.ComponentModel;
using System.IO;
using System.Linq;

Expand Down Expand Up @@ -49,7 +50,7 @@ public class CodeFileBuilder

public ICodeFileBuilderSymbolOrderProvider SymbolOrderProvider { get; set; } = new CodeFileBuilderSymbolOrderProvider();

public const string CurrentVersion = "22";
public const string CurrentVersion = "23";

private IEnumerable<INamespaceSymbol> EnumerateNamespaces(IAssemblySymbol assemblySymbol)
{
Expand Down Expand Up @@ -88,12 +89,12 @@ public CodeFile Build(IAssemblySymbol assemblySymbol, bool runAnalysis, List<Dep
{
foreach (var namedTypeSymbol in SymbolOrderProvider.OrderTypes(namespaceSymbol.GetTypeMembers()))
{
BuildType(builder, namedTypeSymbol, navigationItems);
BuildType(builder, namedTypeSymbol, navigationItems, false);
}
}
else
{
Build(builder, namespaceSymbol, navigationItems);
BuildNamespace(builder, namespaceSymbol, navigationItems);
}
}

Expand Down Expand Up @@ -143,8 +144,14 @@ public static void BuildDependencies(CodeFileTokensBuilder builder, List<Depende
}
}

private void Build(CodeFileTokensBuilder builder, INamespaceSymbol namespaceSymbol, List<NavigationItem> navigationItems)
private void BuildNamespace(CodeFileTokensBuilder builder, INamespaceSymbol namespaceSymbol, List<NavigationItem> navigationItems)
{
bool isHidden = HasOnlyHiddenTypes(namespaceSymbol);

if (isHidden)
{
builder.Append(null, CodeFileTokenKind.HiddenApiRangeStart);
}
builder.Keyword(SyntaxKind.NamespaceKeyword);
builder.Space();
BuildNamespaceName(builder, namespaceSymbol);
Expand All @@ -157,7 +164,7 @@ private void Build(CodeFileTokensBuilder builder, INamespaceSymbol namespaceSymb
List<NavigationItem> namespaceItems = new List<NavigationItem>();
foreach (var namedTypeSymbol in SymbolOrderProvider.OrderTypes(namespaceSymbol.GetTypeMembers()))
{
BuildType(builder, namedTypeSymbol, namespaceItems);
BuildType(builder, namedTypeSymbol, namespaceItems, isHidden);
}

CloseBrace(builder);
Expand All @@ -167,9 +174,14 @@ private void Build(CodeFileTokensBuilder builder, INamespaceSymbol namespaceSymb
NavigationId = namespaceSymbol.GetId(),
Text = namespaceSymbol.ToDisplayString(),
ChildItems = namespaceItems.ToArray(),
Tags = { { "TypeKind", "namespace" } }
Tags = { { "TypeKind", "namespace" } },
IsHiddenApi = isHidden
};
navigationItems.Add(namespaceItem);
if (isHidden)
{
builder.Append(null, CodeFileTokenKind.HiddenApiRangeEnd);
}
}

private void BuildNamespaceName(CodeFileTokensBuilder builder, INamespaceSymbol namespaceSymbol)
Expand All @@ -184,24 +196,36 @@ private void BuildNamespaceName(CodeFileTokensBuilder builder, INamespaceSymbol

private bool HasAnyPublicTypes(INamespaceSymbol subNamespaceSymbol)
{
return subNamespaceSymbol.GetTypeMembers().Any(t => IsAccessible(t));
return subNamespaceSymbol.GetTypeMembers().Any(IsAccessible);
}

private void BuildType(CodeFileTokensBuilder builder, INamedTypeSymbol namedType, List<NavigationItem> navigationBuilder)
private bool HasOnlyHiddenTypes(INamespaceSymbol namespaceSymbol)
{
return namespaceSymbol.GetTypeMembers().All(t=> IsHiddenFromIntellisense(t) || !IsAccessible(t));
}

private void BuildType(CodeFileTokensBuilder builder, INamedTypeSymbol namedType, List<NavigationItem> navigationBuilder, bool inHiddenScope)
{
if (!IsAccessible(namedType))
{
return;
}

bool isHidden = IsHiddenFromIntellisense(namedType);
var navigationItem = new NavigationItem()
{
NavigationId = namedType.GetId(),
Text = namedType.ToDisplayString(SymbolDisplayFormat.MinimallyQualifiedFormat),
IsHiddenApi = isHidden
};
navigationBuilder.Add(navigationItem);
navigationItem.Tags.Add("TypeKind", namedType.TypeKind.ToString().ToLowerInvariant());

if (isHidden && !inHiddenScope)
{
builder.Append(null, CodeFileTokenKind.HiddenApiRangeStart);
}

BuildDocumentation(builder, namedType);
BuildAttributes(builder, namedType.GetAttributes());

Expand Down Expand Up @@ -255,7 +279,7 @@ private void BuildType(CodeFileTokensBuilder builder, INamedTypeSymbol namedType

foreach (var namedTypeSymbol in SymbolOrderProvider.OrderTypes(namedType.GetTypeMembers()))
{
BuildType(builder, namedTypeSymbol, navigationBuilder);
BuildType(builder, namedTypeSymbol, navigationBuilder, inHiddenScope || isHidden);
}

foreach (var member in SymbolOrderProvider.OrderMembers(namedType.GetMembers()))
Expand All @@ -272,10 +296,16 @@ private void BuildType(CodeFileTokensBuilder builder, INamedTypeSymbol namedType
continue;
}
}
BuildMember(builder, member);

BuildMember(builder, member, inHiddenScope);
}

CloseBrace(builder);

if (isHidden && !inHiddenScope)
{
builder.Append(null, CodeFileTokenKind.HiddenApiRangeEnd);
}
}

private void BuildDocumentation(CodeFileTokensBuilder builder, ISymbol symbol)
Expand Down Expand Up @@ -363,8 +393,15 @@ private static void CloseBrace(CodeFileTokensBuilder builder)
builder.NewLine();
}

private void BuildMember(CodeFileTokensBuilder builder, ISymbol member)
private void BuildMember(CodeFileTokensBuilder builder, ISymbol member, bool inHiddenScope)
{
bool isHidden = IsHiddenFromIntellisense(member);

if (isHidden && !inHiddenScope)
{
builder.Append(null, CodeFileTokenKind.HiddenApiRangeStart);
}

BuildDocumentation(builder, member);
BuildAttributes(builder, member.GetAttributes());

Expand All @@ -381,6 +418,10 @@ private void BuildMember(CodeFileTokensBuilder builder, ISymbol member)
}

builder.NewLine();
if (isHidden && !inHiddenScope)
{
builder.Append(null, CodeFileTokenKind.HiddenApiRangeEnd);
}
}

private void BuildAttributes(CodeFileTokensBuilder builder, ImmutableArray<AttributeData> attributes)
Expand Down Expand Up @@ -453,12 +494,17 @@ private bool IsSkippedAttribute(INamedTypeSymbol attributeAttributeClass)
case "IteratorStateMachineAttribute":
case "DefaultMemberAttribute":
case "AsyncIteratorStateMachineAttribute":
case "EditorBrowsableAttribute":
return true;
default:
return false;
}
}

private bool IsHiddenFromIntellisense(ISymbol member) =>
member.GetAttributes().Any(d => d.AttributeClass?.Name == "EditorBrowsableAttribute"
&& (EditorBrowsableState) d.ConstructorArguments[0].Value == EditorBrowsableState.Never);

private void BuildTypedConstant(CodeFileTokensBuilder builder, TypedConstant typedConstant)
{
if (typedConstant.IsNull)
Expand Down
4 changes: 3 additions & 1 deletion src/dotnet/APIView/APIView/Model/CodeFileTokenKind.cs
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,8 @@ public enum CodeFileTokenKind
TableCellEnd = 26,
LeafSectionPlaceholder = 27,
ExternalLinkStart = 28,
ExternalLinkEnd = 29
ExternalLinkEnd = 29,
HiddenApiRangeStart = 30,
HiddenApiRangeEnd = 31
}
}
2 changes: 2 additions & 0 deletions src/dotnet/APIView/APIView/Model/NavigationItem.cs
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@ public class NavigationItem

public Dictionary<string, string> Tags { get; set; } = new Dictionary<string, string>(0);

public bool IsHiddenApi { get; set; }

public override string ToString() => Text;
}
}
9 changes: 9 additions & 0 deletions src/dotnet/APIView/APIViewWeb/Client/css/site.scss
Original file line number Diff line number Diff line change
Expand Up @@ -757,6 +757,15 @@ code {
text-decoration: line-through;
}

.hidden-api .code-inner {
color: var(--text-color-muted) !important;
font-style: italic;
span, a {
JoshLove-msft marked this conversation as resolved.
Show resolved Hide resolved
color: var(--text-color-muted) !important;
font-style: italic;
}
}

.file-code-icon {
filter: var(--icon-color-filter);
}
Expand Down
19 changes: 17 additions & 2 deletions src/dotnet/APIView/APIViewWeb/Client/src/review.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,15 +9,22 @@ $(() => {
const SHOW_DIFFONLY_CHECKBOX = ".show-diffonly-checkbox";
const SHOW_DIFFONLY_HREF = ".show-diffonly";
const TOGGLE_DOCUMENTATION = ".line-toggle-documentation-button";
const SEL_HIDDEN_CLASS = ".hidden-api-toggleable";
const SHOW_HIDDEN_CHECK_COMPONENT = "#show-hidden-api-component";
const SHOW_HIDDEN_CHECKBOX = "#show-hidden-api-checkbox";
const SHOW_HIDDEN_HREF = ".show-hidden-api";

hideCheckboxIfNoDocs();
hideCheckboxesIfNotApplicable();

/* FUNCTIONS
--------------------------------------------------------------------------------------------------------------------------------------------------------*/
function hideCheckboxIfNoDocs() {
function hideCheckboxesIfNotApplicable() {
if ($(SEL_DOC_CLASS).length == 0) {
$(SHOW_DOC_CHECK_COMPONENT).hide();
}
if ($(SEL_HIDDEN_CLASS).length == 0) {
$(SHOW_HIDDEN_CHECK_COMPONENT).hide();
}
}

/* Split left and right review panes using split.js */
Expand Down Expand Up @@ -337,6 +344,14 @@ $(() => {
$(SHOW_DOC_CHECKBOX)[0].click();
});

$(SHOW_HIDDEN_CHECKBOX).on("click", e => {
$(SEL_HIDDEN_CLASS).toggleClass("d-none");
});

$(SHOW_HIDDEN_HREF).on("click", e => {
$(SHOW_HIDDEN_CHECKBOX)[0].click();
});

$(SHOW_DIFFONLY_CHECKBOX).on("click", e => {
$(SHOW_DIFFONLY_HREF)[0].click();
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -196,6 +196,12 @@
<label>&nbsp;Show Documentation</label>
</a>
</span>
<span class="dropdown-item checkbox" id="show-hidden-api-component">
<label>
<input type="checkbox" id="show-hidden-api-checkbox">
&nbsp;Show Hidden APIs
</label>
</span>
<span class="dropdown-item checkbox">
<label>
@if (userPreference.HideLineNumbers == true)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
bool isHeadingWithDelta = false;
string headingClass = String.Empty;
string documentationRow = String.Empty;
string hiddenApiRow = String.Empty;
string codeLineDisplay = String.Empty;
string codeLineClass = (!String.IsNullOrWhiteSpace(Model.CodeLine.LineClass)) ? Model.CodeLine.LineClass.Trim() : String.Empty;
int? sectionKey = Model.DiffSectionId ?? Model.CodeLine.SectionKey;
Expand Down Expand Up @@ -58,6 +59,15 @@
codeLineDisplay = "hidden-row";
}

if (Model.CodeLine.IsHiddenApi)
{
hiddenApiRow = "hidden-api";
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This applies the italicized style.

if (Model.Kind == DiffLineKind.Unchanged)
{
hiddenApiRow += " hidden-api-toggleable d-none";
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This makes it toggleable. When there is a diff, it should not be toggleable.

}
}

if (Regex.IsMatch(codeLineClass, @".*lvl_[0-9]+_parent.*"))
{
isSubSectionHeading = true;
Expand All @@ -70,7 +80,7 @@
isHeadingWithDelta = true;
}
var userPreference = TempData["UserPreference"] as UserPreferenceModel ?? new UserPreferenceModel();
var rowClass = RemoveMultipleSpaces($"code-line {headingClass} {codeLineClass} {lineClass} {codeLineDisplay} {documentationRow}");
var rowClass = RemoveMultipleSpaces($"code-line {headingClass} {codeLineClass} {lineClass} {codeLineDisplay} {documentationRow} {hiddenApiRow}");
var cellContent = String.Empty;
for (int i = 0; i < Model.CodeLine.Indent; i++)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
<ul class="nav-list-children">
@foreach (var item in Model)
{
<li class="@navListGroupClass">
<li class="@navListGroupClass @(item.IsHiddenApi ? " hidden-api-toggleable d-none" : "")">
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This hides the navigation section if it represents a hidden namespace/type.

JoshLove-msft marked this conversation as resolved.
Show resolved Hide resolved
<span class="nav-list-toggle @(item.ChildItems.Any() ? "":"invisible")"></span>

@if (item.Tags != null && item.Tags.ContainsKey("TypeKind"))
Expand Down