Skip to content

Commit 5aa6d43

Browse files
jnthntatumcopybara-github
authored andcommitted
Add support for estimating original source positions.
Ported from the go implementation. PiperOrigin-RevId: 934581307
1 parent eb8a894 commit 5aa6d43

12 files changed

Lines changed: 597 additions & 24 deletions

common/source.cc

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919
#include <cstdint>
2020
#include <limits>
2121
#include <memory>
22+
#include <optional>
2223
#include <string>
2324
#include <tuple>
2425
#include <utility>

policy/BUILD

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -84,6 +84,7 @@ cc_library(
8484
":cel_policy_parser",
8585
"//common:source",
8686
"//internal:status_macros",
87+
"//policy/internal:yaml_string_element_scanner",
8788
"@com_google_absl//absl/status",
8889
"@com_google_absl//absl/status:statusor",
8990
"@com_google_absl//absl/strings",
@@ -190,6 +191,7 @@ cc_test(
190191
":compiler",
191192
":yaml_policy_parser",
192193
"//common:ast",
194+
"//common:ast_proto",
193195
"//common:decl",
194196
"//common:navigable_ast",
195197
"//common:source",
@@ -211,6 +213,7 @@ cc_test(
211213
"//runtime:runtime_builder",
212214
"//runtime:runtime_options",
213215
"//runtime:standard_runtime_builder_factory",
216+
"//tools:cel_unparser",
214217
"@com_google_absl//absl/status",
215218
"@com_google_absl//absl/status:status_matchers",
216219
"@com_google_absl//absl/status:statusor",

policy/cel_policy.cc

Lines changed: 39 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -59,34 +59,64 @@ std::string IndentBlock(absl::string_view text) {
5959

6060
void CelPolicySource::NoteSourcePosition(CelPolicyElementId id,
6161
SourcePosition position) {
62-
source_positions_[id] = position;
62+
source_info_[id].position = position;
63+
}
64+
65+
void CelPolicySource::NoteSourceRange(CelPolicyElementId id,
66+
std::optional<SourceRange> range,
67+
bool quoted) {
68+
ElementSourceInfo& info = source_info_[id];
69+
info.range = range;
70+
info.quoted = quoted;
71+
if (range.has_value() && info.position == -1) {
72+
info.position = range->begin;
73+
}
6374
}
6475

6576
std::optional<SourcePosition> CelPolicySource::GetSourcePosition(
6677
CelPolicyElementId id) const {
67-
auto it = source_positions_.find(id);
68-
if (it == source_positions_.end()) {
78+
auto it = source_info_.find(id);
79+
if (it == source_info_.end() || it->second.position == -1) {
6980
return std::nullopt;
7081
}
71-
return it->second;
82+
return it->second.position;
83+
}
84+
85+
std::optional<SourceRange> CelPolicySource::GetSourceRange(
86+
CelPolicyElementId id) const {
87+
auto it = source_info_.find(id);
88+
if (it == source_info_.end() || !it->second.range.has_value()) {
89+
return std::nullopt;
90+
}
91+
return it->second.range;
92+
}
93+
94+
std::optional<bool> CelPolicySource::IsQuoted(CelPolicyElementId id) const {
95+
auto it = source_info_.find(id);
96+
if (it == source_info_.end() || !it->second.range.has_value()) {
97+
return std::nullopt;
98+
}
99+
return it->second.quoted;
72100
}
73101

74102
std::optional<SourceLocation> CelPolicySource::GetSourceLocation(
75103
CelPolicyElementId id) const {
76-
auto it = source_positions_.find(id);
77-
if (it == source_positions_.end()) {
104+
auto it = source_info_.find(id);
105+
if (it == source_info_.end() || it->second.position == -1) {
78106
return std::nullopt;
79107
}
80-
return policy_source_->GetLocation(it->second);
108+
return policy_source_->GetLocation(it->second.position);
81109
}
82110

83111
std::string CelPolicySource::DebugString() const {
84112
std::string result;
85113

86114
// Sort the source elements in descending order of position
87115
std::vector<std::pair<CelPolicyElementId, SourcePosition>> sorted_positions;
88-
for (const auto& pair : source_positions_) {
89-
sorted_positions.push_back(pair);
116+
for (const auto& [id, info] : source_info_) {
117+
if (info.position != -1) {
118+
sorted_positions.push_back({id, info.position});
119+
}
90120
}
91121
std::sort(sorted_positions.begin(), sorted_positions.end(),
92122
[](const auto& a, const auto& b) {

policy/cel_policy.h

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,12 @@ namespace cel {
3434

3535
using CelPolicyElementId = int32_t;
3636

37+
struct ElementSourceInfo {
38+
SourcePosition position = -1;
39+
std::optional<SourceRange> range;
40+
bool quoted = false;
41+
};
42+
3743
class CelPolicySource {
3844
public:
3945
explicit CelPolicySource(cel::SourcePtr policy_source)
@@ -43,15 +49,22 @@ class CelPolicySource {
4349

4450
void NoteSourcePosition(CelPolicyElementId id, SourcePosition position);
4551

52+
void NoteSourceRange(CelPolicyElementId id, std::optional<SourceRange> range,
53+
bool quoted);
54+
4655
std::optional<SourcePosition> GetSourcePosition(CelPolicyElementId id) const;
4756

57+
std::optional<SourceRange> GetSourceRange(CelPolicyElementId id) const;
58+
59+
std::optional<bool> IsQuoted(CelPolicyElementId id) const;
60+
4861
std::optional<SourceLocation> GetSourceLocation(CelPolicyElementId id) const;
4962

5063
std::string DebugString() const;
5164

5265
private:
5366
cel::SourcePtr policy_source_;
54-
absl::flat_hash_map<CelPolicyElementId, SourcePosition> source_positions_;
67+
absl::flat_hash_map<CelPolicyElementId, ElementSourceInfo> source_info_;
5568
};
5669

5770
class ValueString {

policy/cel_policy_parse_result.cc

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -52,8 +52,12 @@ std::string CelPolicyIssue::ToDisplayString(
5252
std::string snippet;
5353
if (source != nullptr) {
5454
if (relative_position_) {
55+
std::optional<SourceRange> range = source->GetSourceRange(element_id_);
5556
std::optional<SourcePosition> base =
5657
source->GetSourcePosition(element_id_);
58+
if (range.has_value()) {
59+
base = range->begin;
60+
}
5761
if (element_id_ == -1) {
5862
base.emplace(0);
5963
}

policy/compiler.cc

Lines changed: 71 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919
#include <memory>
2020
#include <optional>
2121
#include <string>
22+
#include <type_traits>
2223
#include <utility>
2324
#include <variant>
2425
#include <vector>
@@ -157,11 +158,19 @@ class IntermediateCompiledPolicy {
157158
void set_semantics(RuleSemantics semantics) { semantics_ = semantics; }
158159
RuleSemantics semantics() const { return semantics_; }
159160

161+
void set_policy_source(const CelPolicySource* absl_nullable src) {
162+
policy_source_ = src;
163+
}
164+
const CelPolicySource* absl_nullable policy_source() const {
165+
return policy_source_;
166+
}
167+
160168
private:
161169
std::string name_;
162170
std::string display_name_;
163171
std::string description_;
164172
RuleSemantics semantics_ = RuleSemantics::kFirstMatch;
173+
const CelPolicySource* absl_nullable policy_source_ = nullptr;
165174

166175
CompiledRule root_rule_;
167176
};
@@ -315,6 +324,33 @@ class PolicyCompiler {
315324
return src_->content()->description();
316325
}
317326

327+
absl::StatusOr<ValidationResult> CompileExpression(CelPolicyElementId id,
328+
absl::string_view val,
329+
const Compiler* env) {
330+
std::unique_ptr<cel::Source> source;
331+
if (src_ != nullptr && src_->content() != nullptr) {
332+
std::optional<SourceRange> range;
333+
range = src_->GetSourceRange(id);
334+
bool use_subrange = !(src_->IsQuoted(id).value_or(true));
335+
if (range.has_value() && use_subrange) {
336+
source = std::make_unique<SourceSubrange>(*src_->content(), *range);
337+
}
338+
}
339+
340+
if (source == nullptr) {
341+
// For quoted strings, the source should be generated from the interpreted
342+
// YAML value.
343+
CEL_ASSIGN_OR_RETURN(
344+
source, cel::NewSource(val, std::string(GetSourceDescription())));
345+
}
346+
auto result = env->Compile(*source, &arena_);
347+
if (!result.ok()) {
348+
return result;
349+
}
350+
result->SetSource(std::move(source));
351+
return result;
352+
}
353+
318354
void AdaptTypeCheckIssues(CelPolicyElementId id, const ValidationResult& r) {
319355
const Source* source = r.GetSource();
320356

@@ -336,8 +372,8 @@ class PolicyCompiler {
336372
const cel::OutputBlock& output_block, const Compiler* env) {
337373
CompiledOutputBlock output;
338374
CEL_ASSIGN_OR_RETURN(auto output_validation,
339-
env->Compile(output_block.output().value(),
340-
GetSourceDescription(), &arena_));
375+
CompileExpression(output_block.output().id(),
376+
output_block.output().value(), env));
341377
AdaptTypeCheckIssues(output_block.output().id(), output_validation);
342378

343379
cel::Type result_type = DynType();
@@ -352,9 +388,10 @@ class PolicyCompiler {
352388
}
353389
}
354390
if (output_block.explanation().has_value()) {
355-
CEL_ASSIGN_OR_RETURN(auto explanation_validation,
356-
env->Compile(output_block.explanation()->value(),
357-
GetSourceDescription(), &arena_));
391+
CEL_ASSIGN_OR_RETURN(
392+
auto explanation_validation,
393+
CompileExpression(output_block.explanation()->id(),
394+
output_block.explanation()->value(), env));
358395
AdaptTypeCheckIssues(output_block.explanation()->id(),
359396
explanation_validation);
360397
if (explanation_validation.IsValid()) {
@@ -378,8 +415,8 @@ class PolicyCompiler {
378415
c_match.id = match.id();
379416
if (match.condition().has_value()) {
380417
CEL_ASSIGN_OR_RETURN(auto validation,
381-
env->Compile(match.condition()->value(),
382-
GetSourceDescription(), &arena_));
418+
CompileExpression(match.condition()->id(),
419+
match.condition()->value(), env));
383420
AdaptTypeCheckIssues(match.condition()->id(), validation);
384421
if (validation.IsValid()) {
385422
CEL_ASSIGN_OR_RETURN(auto ast, validation.ReleaseAst());
@@ -422,9 +459,10 @@ class PolicyCompiler {
422459
continue;
423460
}
424461
std::string ident = absl::StrCat("variables.", name);
425-
CEL_ASSIGN_OR_RETURN(auto validation,
426-
env->Compile(variable.expression().value(),
427-
GetSourceDescription(), &arena_));
462+
CEL_ASSIGN_OR_RETURN(
463+
auto validation,
464+
CompileExpression(variable.expression().id(),
465+
variable.expression().value(), env));
428466
AdaptTypeCheckIssues(variable.expression().id(), validation);
429467
if (!validation.IsValid()) {
430468
continue;
@@ -480,6 +518,7 @@ class PolicyCompiler {
480518
absl::Status CompilePolicy(const CelPolicy& policy,
481519
IntermediateCompiledPolicy* out) {
482520
src_ = policy.source();
521+
out->set_policy_source(src_);
483522
out->set_semantics(RuleSemantics::kFirstMatch);
484523
out->set_name(policy.name().value());
485524
out->set_display_name(
@@ -513,6 +552,21 @@ class FirstMatchComposer {
513552
std::unique_ptr<cel::Ast> ReleaseAst() { return std::move(ast_); }
514553

515554
private:
555+
SourcePosition GetAstOffset(CelPolicyElementId id) const {
556+
if (icp_.policy_source() == nullptr) {
557+
return 0;
558+
}
559+
if (auto range = icp_.policy_source()->GetSourceRange(id);
560+
range.has_value()) {
561+
return range->begin;
562+
}
563+
if (auto pos = icp_.policy_source()->GetSourcePosition(id);
564+
pos.has_value()) {
565+
return *pos;
566+
}
567+
return 0;
568+
}
569+
516570
using VariableScope = absl::flat_hash_map<std::string, int>;
517571

518572
std::optional<int> ResolvePolicyVariable(absl::string_view reference);
@@ -733,7 +787,8 @@ absl::StatusOr<bool> FirstMatchComposer::ComposeRule(const CompiledRule& rule,
733787
MapVariables(condition);
734788
factory_.StartCopyContext();
735789
auto copy = factory_.Copy(condition.root_expr());
736-
auto source_info = factory_.RemapSourceInfo(condition.source_info());
790+
auto source_info = factory_.RemapSourceInfo(
791+
condition.source_info(), GetAstOffset(match.condition->id));
737792
factory_.MergeSourceInfo(source_info);
738793
*insertion_point = factory_.NewCall("_?_:_", std::move(copy));
739794
insertion_point->mutable_call_expr().mutable_args().push_back(
@@ -792,7 +847,8 @@ absl::StatusOr<bool> FirstMatchComposer::ComposeProduction(
792847
MapVariables(ast);
793848
factory_.StartCopyContext();
794849
Expr to_insert = factory_.Copy(ast.root_expr());
795-
auto source_info = factory_.RemapSourceInfo(ast.source_info());
850+
auto source_info =
851+
factory_.RemapSourceInfo(ast.source_info(), GetAstOffset(output_ast.id));
796852
factory_.MergeSourceInfo(source_info);
797853
insertion_expr = std::move(to_insert);
798854

@@ -832,8 +888,9 @@ void FirstMatchComposer::ComposeRuleVariables(const CompiledRule& rule,
832888
MapVariables(ast);
833889
factory_.StartCopyContext();
834890
auto insertion = factory_.Copy(ast.root_expr());
835-
// TODO(b/506179116): apply the position offsets here.
836-
auto info = factory_.RemapSourceInfo(ast.source_info());
891+
auto info = factory_.RemapSourceInfo(ast.source_info(),
892+
GetAstOffset(variable.ast.id));
893+
factory_.MergeSourceInfo(info);
837894
ABSL_DCHECK(init.has_list_expr());
838895
int index = init.mutable_list_expr().elements().size();
839896
init.mutable_list_expr().mutable_elements().push_back(

0 commit comments

Comments
 (0)