Skip to content

Commit

Permalink
Refactor out GetPartitionAlignmentSubranges() for future alignment fu…
Browse files Browse the repository at this point in the history
…nctions.

This is useful anywhere where sub-ranges needed to be chosen from a range of partitions that contains heterogenous syntax tree node types, e.g. statements, class/module body items.

No functional change.

issues #357, #358, #319

PiperOrigin-RevId: 327521152
  • Loading branch information
fangism authored and hzeller committed Aug 19, 2020
1 parent f6f516f commit bee424a
Show file tree
Hide file tree
Showing 5 changed files with 183 additions and 33 deletions.
1 change: 1 addition & 0 deletions common/formatting/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ cc_test(
":token_partition_tree",
":unwrapped_line_test_utils",
"//common/text:tree_builder_test_util",
"//common/util:range",
"//common/util:spacer",
"@com_google_absl//absl/strings",
"@com_google_googletest//:gtest_main",
Expand Down
38 changes: 38 additions & 0 deletions common/formatting/align.cc
Original file line number Diff line number Diff line change
Expand Up @@ -660,4 +660,42 @@ void TabularAlignTokens(TokenPartitionTree* partition_ptr,
VLOG(1) << "end of " << __FUNCTION__;
}

std::vector<TokenPartitionRange> GetPartitionAlignmentSubranges(
const TokenPartitionRange& partitions,
const std::function<AlignmentGroupAction(const TokenPartitionTree&)>&
partition_selector,
int min_match_count) {
std::vector<TokenPartitionRange> result;

// Grab ranges of consecutive data declarations with >= 2 elements.
int match_count = 0;
auto last_range_start = partitions.begin();
for (auto iter = last_range_start; iter != partitions.end(); ++iter) {
switch (partition_selector(*iter)) {
case AlignmentGroupAction::kIgnore:
continue;
case AlignmentGroupAction::kMatch: {
if (match_count == 0) {
// This is the start of a new range of interest.
last_range_start = iter;
}
++match_count;
break;
}
case AlignmentGroupAction::kNoMatch: {
if (match_count >= min_match_count) {
result.emplace_back(last_range_start, iter);
}
match_count = 0; // reset
break;
}
} // switch
} // for
// Flush out the last range.
if (match_count >= min_match_count) {
result.emplace_back(last_range_start, partitions.end());
}
return result;
}

} // namespace verible
38 changes: 38 additions & 0 deletions common/formatting/align.h
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,44 @@ class ColumnSchemaScanner : public TreeContextPathVisitor {
std::vector<ColumnPositionEntry> sparse_columns_;
};

// This enum drives partition sub-range selection in the
// GetPartitionAlignmentSubranges() function.
enum class AlignmentGroupAction {
kIgnore, // This does not influence the current matching range.
kMatch, // Include this partition in the current matching range.
kNoMatch, // Close the current matching range (if any).
};

// From a range of token 'partitions', this selects sub-ranges to align.
// 'partition_selector' decides which partitions qualify for alignment.
// 'min_match_count' sets the minimum sub-range size to return.
//
// Visualization from 'partition_selector's perspective:
//
// case 1:
// nomatch
// match // not enough matches to yield a group for min_match_count=2
// nomatch
//
// case 2:
// nomatch
// match // an alignment group starts here
// match // ends here, inclusively
// nomatch
//
// case 3:
// nomatch
// match // an alignment group starts here
// ignore // ... continues ...
// match // ends here, inclusively
// nomatch
//
std::vector<TokenPartitionRange> GetPartitionAlignmentSubranges(
const TokenPartitionRange& partitions,
const std::function<AlignmentGroupAction(const TokenPartitionTree&)>&
partition_selector,
int min_match_count = 2);

// This is the interface used to extract alignment cells from ranges of tokens.
// Note that it is not required to use a ColumnSchemaScanner.
using AlignmentCellScannerFunction =
Expand Down
92 changes: 92 additions & 0 deletions common/formatting/align_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -21,11 +21,14 @@
#include "common/formatting/token_partition_tree.h"
#include "common/formatting/unwrapped_line_test_utils.h"
#include "common/text/tree_builder_test_util.h"
#include "common/util/range.h"
#include "common/util/spacer.h"

namespace verible {
namespace {

using ::testing::ElementsAre;

// Helper class that initializes an array of tokens to be partitioned
// into TokenPartitionTree.
class AlignmentTestFixture : public ::testing::Test,
Expand Down Expand Up @@ -560,5 +563,94 @@ TEST_F(MultiAlignmentGroupTest, BlankLineSeparatedGroups) {

// TODO(fangism): test case for demonstrating flush-right

class GetPartitionAlignmentSubrangesTestFixture : public AlignmentTestFixture {
public:
GetPartitionAlignmentSubrangesTestFixture()
: AlignmentTestFixture(
"ignore match nomatch match match match nomatch nomatch match "
"ignore match"),
syntax_tree_(TNode(
1, // one token per partition for simplicity
TNode(2, Leaf(1, tokens_[0])), //
TNode(2, Leaf(1, tokens_[1])), // singleton range too short here
TNode(2, Leaf(1, tokens_[2])), //
TNode(2, Leaf(1, tokens_[3])), // expect match from here
TNode(2, Leaf(1, tokens_[4])), // ...
TNode(2, Leaf(1, tokens_[5])), // ... to here (inclusive)
TNode(2, Leaf(1, tokens_[6])), //
TNode(2, Leaf(1, tokens_[7])), //
TNode(2, Leaf(1, tokens_[8])), // and from here to the end().
TNode(2, Leaf(1, tokens_[9])), // ...
TNode(2, Leaf(1, tokens_[10])))), // ...
partition_(/* temporary */ UnwrappedLine()) {
// Establish format token ranges per partition.
const auto begin = pre_format_tokens_.begin();
UnwrappedLine all(0, begin);
all.SpanUpToToken(pre_format_tokens_.end());
all.SetOrigin(&*syntax_tree_);

std::vector<UnwrappedLine> uwlines;
for (int i = 0; i < pre_format_tokens_.size(); ++i) {
uwlines.emplace_back(0, begin + i);
uwlines.back().SpanUpToToken(begin + i + 1);
uwlines.back().SetOrigin(
DescendPath(*syntax_tree_, {static_cast<size_t>(i)}));
}

// Construct 2-level token partition.
using tree_type = TokenPartitionTree;
partition_ = tree_type{
all,
tree_type{uwlines[0]},
tree_type{uwlines[1]}, // one match not enough
tree_type{uwlines[2]},
tree_type{uwlines[3]}, // start of match
tree_type{uwlines[4]}, // ...
tree_type{uwlines[5]}, // ...
tree_type{uwlines[6]}, // end of match
tree_type{uwlines[7]},
tree_type{uwlines[8]}, // start of match
tree_type{uwlines[9]}, // ...
tree_type{uwlines[10]}, // ...
};
}

protected:
static AlignmentGroupAction PartitionSelector(
const TokenPartitionTree& partition) {
const absl::string_view text =
partition.Value().TokensRange().front().Text();
if (text == "match") {
return AlignmentGroupAction::kMatch;
} else if (text == "nomatch") {
return AlignmentGroupAction::kNoMatch;
} else {
return AlignmentGroupAction::kIgnore;
}
}

protected:
// Syntax tree from which token partition originates.
SymbolPtr syntax_tree_;

// Format token partitioning (what would be the result of TreeUnwrapper).
TokenPartitionTree partition_;
};

TEST_F(GetPartitionAlignmentSubrangesTestFixture, VariousRanges) {
const TokenPartitionRange children(partition_.Children().begin(),
partition_.Children().end());

const std::vector<TokenPartitionRange> ranges(
GetPartitionAlignmentSubranges(children, &PartitionSelector));

using P = std::pair<int, int>;
std::vector<P> range_indices;
for (const auto& range : ranges) {
range_indices.push_back(SubRangeIndices(range, children));
}
EXPECT_THAT(range_indices, ElementsAre(P(3, 6), P(8, 11)));
}

} // namespace
} // namespace verible
47 changes: 14 additions & 33 deletions verilog/formatting/align.cc
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ namespace formatter {

using verible::AlignmentCellScannerGenerator;
using verible::AlignmentColumnProperties;
using verible::AlignmentGroupAction;
using verible::ByteOffsetSet;
using verible::ColumnSchemaScanner;
using verible::down_cast;
Expand Down Expand Up @@ -342,40 +343,20 @@ static bool IsAlignableDeclaration(const SyntaxTreeNode& node) {
}

static std::vector<TokenPartitionRange> GetConsecutiveDataDeclarationGroups(
const TokenPartitionRange& outer_partition_bounds) {
const TokenPartitionRange& partitions) {
VLOG(2) << __FUNCTION__;
std::vector<TokenPartitionRange> result;

// Grab ranges of consecutive data declarations with >= 2 elements.
int consecutive_count = 0;
auto last_range_start = outer_partition_bounds.begin();
for (auto iter = last_range_start; iter != outer_partition_bounds.end();
++iter) {
const Symbol* origin = iter->Value().Origin();
if (origin == nullptr) continue;
const verible::SymbolTag symbol_tag = origin->Tag();
if (symbol_tag.kind != verible::SymbolKind::kNode) continue;
const SyntaxTreeNode& node = verible::SymbolCastToNode(*origin);
if (IsAlignableDeclaration(node)) {
if (consecutive_count == 0) {
// This is the start of a new range of interest.
last_range_start = iter;
}
++consecutive_count;
} else {
if (consecutive_count >= 2) {
result.emplace_back(last_range_start, iter);
}
consecutive_count = 0; // reset
}
}
// Flush out the last range.
if (consecutive_count >= 2) {
result.emplace_back(last_range_start, outer_partition_bounds.end());
}

VLOG(2) << "end of " << __FUNCTION__;
return result;
return GetPartitionAlignmentSubranges(
partitions, //
[](const TokenPartitionTree& partition) -> AlignmentGroupAction {
const Symbol* origin = partition.Value().Origin();
if (origin == nullptr) return AlignmentGroupAction::kIgnore;
const verible::SymbolTag symbol_tag = origin->Tag();
if (symbol_tag.kind != verible::SymbolKind::kNode)
return AlignmentGroupAction::kIgnore;
const SyntaxTreeNode& node = verible::SymbolCastToNode(*origin);
return IsAlignableDeclaration(node) ? AlignmentGroupAction::kMatch
: AlignmentGroupAction::kNoMatch;
});
}

// Much of the implementation of this scanner was based on
Expand Down

0 comments on commit bee424a

Please sign in to comment.