From cefb512737113cb4d364f33ecc9b1643c8f95eae Mon Sep 17 00:00:00 2001 From: Jonathan Tatum Date: Thu, 18 Jun 2026 15:37:19 -0700 Subject: [PATCH] Add support for creating a subspan view of a cel::Source. PiperOrigin-RevId: 934581306 --- common/source.cc | 45 +++++++++++++++++++++++++++++++ common/source.h | 32 ++++++++++++++++++++++ common/source_test.cc | 39 +++++++++++++++++++++++++++ compiler/BUILD | 1 + compiler/compiler.h | 33 +++++++++++++++++++++-- compiler/compiler_factory.cc | 27 ++++++++----------- compiler/compiler_factory_test.cc | 14 ++++++++++ 7 files changed, 173 insertions(+), 18 deletions(-) diff --git a/common/source.cc b/common/source.cc index 8c32ad6ba..0e0159859 100644 --- a/common/source.cc +++ b/common/source.cc @@ -585,6 +585,51 @@ absl::optional> Source::FindLine( return std::make_pair(line, line_offsets[static_cast(line) - 2]); } +SourceSubrange::SourceSubrange(const Source& source, SourceRange range) + : source_(source), range_(range) { + SourcePosition size = source_.content().size(); + ABSL_DCHECK(range_.begin >= 0); + ABSL_DCHECK(range_.begin <= size); + ABSL_DCHECK(range_.end >= range_.begin); + ABSL_DCHECK(range_.end <= size); + if (range_.begin < 0) { + range_.begin = 0; + } + if (range_.begin > size) { + range_.begin = size; + } + if (range_.end < range_.begin) { + range_.end = range_.begin; + } + if (range_.end > size) { + range_.end = size; + } + for (const auto& line_offset : source_.line_offsets()) { + if (line_offset > range_.begin && line_offset <= range_.end) { + line_offsets_.push_back(line_offset - range_.begin); + } + } + line_offsets_.push_back(range_.end - range_.begin + 1); +} + +SourceContentView SourceSubrange::content() const { + auto parent_content = source_.content(); + if (parent_content.empty() || range_.begin >= range_.end) { + return EmptyContentView(); + } + return absl::visit( + [this](auto view) { + return SourceContentView( + view.subspan(static_cast(range_.begin), + static_cast(range_.end - range_.begin))); + }, + parent_content.view_); +} + +absl::Span SourceSubrange::line_offsets() const { + return absl::MakeConstSpan(line_offsets_); +} + absl::StatusOr NewSource(absl::string_view content, std::string description) { return common_internal::NewSourceImpl(std::move(description), content, diff --git a/common/source.h b/common/source.h index 6453363a8..b3968453f 100644 --- a/common/source.h +++ b/common/source.h @@ -22,6 +22,7 @@ #include "absl/base/attributes.h" #include "absl/base/nullability.h" +#include "absl/container/inlined_vector.h" #include "absl/status/statusor.h" #include "absl/strings/cord.h" #include "absl/strings/string_view.h" @@ -36,6 +37,7 @@ class SourceImpl; } // namespace common_internal class Source; +class SourceSubrange; // SourcePosition represents an offset in source text. using SourcePosition = int32_t; @@ -94,6 +96,7 @@ class SourceContentView final { private: friend class Source; + friend class SourceSubrange; constexpr SourceContentView() = default; @@ -178,6 +181,7 @@ class Source { private: friend class common_internal::SourceImpl; + friend class SourceSubrange; Source() = default; @@ -187,6 +191,34 @@ class Source { SourcePosition position) const; }; +// `SourceSubrange` is a view of a subrange fo an underlying `Source` object. +// Intended to be used when the CEL expression is embedded in a larger text +// representation (such as a.celpolicy). +// +// The parent `Source` must outlive this object. +class SourceSubrange final : public Source { + public: + SourceSubrange(const Source& source ABSL_ATTRIBUTE_LIFETIME_BOUND, + SourceRange range); + + absl::string_view description() const ABSL_ATTRIBUTE_LIFETIME_BOUND override { + return source_.description(); + } + + // Returns a view of the underlying expression text. + ContentView content() const ABSL_ATTRIBUTE_LIFETIME_BOUND override; + + // Returns a `absl::Span` of `SourcePosition` which represent the positions + // where new lines occur. + absl::Span line_offsets() const + ABSL_ATTRIBUTE_LIFETIME_BOUND override; + + private: + const Source& source_; + SourceRange range_; + absl::InlinedVector line_offsets_; +}; + using SourcePtr = std::unique_ptr; absl::StatusOr NewSource( diff --git a/common/source_test.cc b/common/source_test.cc index 2a3b78893..122fb96a9 100644 --- a/common/source_test.cc +++ b/common/source_test.cc @@ -223,5 +223,44 @@ TEST(Source, DisplayErrorLocationFullWidth) { "\n | .．＾"); } +TEST(SourceSubrange, Description) { + ASSERT_OK_AND_ASSIGN(auto source, NewSource("hello world", "subrange-test")); + SourceSubrange subrange(*source, SourceRange{0, 5}); + EXPECT_THAT(subrange.description(), Eq("subrange-test")); +} + +TEST(SourceSubrange, Content) { + ASSERT_OK_AND_ASSIGN(auto source, NewSource("hello world", "subrange-test")); + SourceSubrange subrange(*source, SourceRange{6, 11}); + EXPECT_THAT(subrange.content().ToString(), Eq("world")); +} + +TEST(SourceSubrange, ContentEmpty) { + ASSERT_OK_AND_ASSIGN(auto source, NewSource("hello world", "subrange-test")); + SourceSubrange subrange(*source, SourceRange{5, 5}); + EXPECT_THAT(subrange.content().ToString(), Eq("")); +} + +TEST(SourceSubrange, LineOffsetsNoNewlines) { + ASSERT_OK_AND_ASSIGN(auto source, + NewSource("hello\nworld\n", "subrange-test")); + SourceSubrange subrange(*source, SourceRange{0, 5}); + EXPECT_THAT(subrange.line_offsets(), ElementsAre(6)); +} + +TEST(SourceSubrange, LineOffsetsWithNewlines) { + ASSERT_OK_AND_ASSIGN(auto source, + NewSource("hello\nworld\ncel", "subrange-test")); + SourceSubrange subrange(*source, SourceRange{0, 11}); + EXPECT_THAT(subrange.line_offsets(), ElementsAre(6, 12)); +} + +TEST(SourceSubrange, LineOffsetsMiddleSubrange) { + ASSERT_OK_AND_ASSIGN(auto source, + NewSource("hello\nworld\ncel\ncpp", "subrange-test")); + SourceSubrange subrange(*source, SourceRange{6, 15}); + EXPECT_THAT(subrange.line_offsets(), ElementsAre(6, 10)); +} + } // namespace } // namespace cel diff --git a/compiler/BUILD b/compiler/BUILD index d4a0ab4ac..2e3f7a2ba 100644 --- a/compiler/BUILD +++ b/compiler/BUILD @@ -25,6 +25,7 @@ cc_library( "//checker:type_checker", "//checker:type_checker_builder", "//checker:validation_result", + "//common:source", "//parser:options", "//parser:parser_interface", "//validator", diff --git a/compiler/compiler.h b/compiler/compiler.h index 27237df60..174912f71 100644 --- a/compiler/compiler.h +++ b/compiler/compiler.h @@ -27,6 +27,7 @@ #include "checker/type_checker.h" #include "checker/type_checker_builder.h" #include "checker/validation_result.h" +#include "common/source.h" #include "parser/options.h" #include "parser/parser_interface.h" #include "validator/validator.h" @@ -129,9 +130,18 @@ class Compiler { public: virtual ~Compiler() = default; - virtual absl::StatusOr Compile( + absl::StatusOr Compile( + const Source& source, google::protobuf::Arena* absl_nullable arena) const { + return CompileImpl(source, arena); + } + + absl::StatusOr Compile(const Source& source) const { + return CompileImpl(source, nullptr); + } + + absl::StatusOr Compile( absl::string_view source, absl::string_view description, - google::protobuf::Arena* absl_nullable arena) const = 0; + google::protobuf::Arena* absl_nullable arena) const; absl::StatusOr Compile(absl::string_view source) const { return Compile(source, "", nullptr); @@ -159,8 +169,27 @@ class Compiler { // The returned builder does not share state with the compiler and may be // modified independently. virtual std::unique_ptr ToBuilder() const = 0; + + protected: + virtual absl::StatusOr CompileImpl( + const Source& source, google::protobuf::Arena* absl_nullable arena) const = 0; }; +inline absl::StatusOr Compiler::Compile( + absl::string_view source, absl::string_view description, + google::protobuf::Arena* absl_nullable arena) const { + absl::StatusOr source_obj = + NewSource(source, std::string(description)); + if (!source_obj.ok()) { + return source_obj.status(); + } + absl::StatusOr result = CompileImpl(**source_obj, arena); + if (result.ok()) { + result->SetSource(std::move(*source_obj)); + } + return result; +} + } // namespace cel #endif // THIRD_PARTY_CEL_CPP_COMPILER_COMPILER_INTERFACE_H_ diff --git a/compiler/compiler_factory.cc b/compiler/compiler_factory.cc index ed22c5630..a8a44461c 100644 --- a/compiler/compiler_factory.cc +++ b/compiler/compiler_factory.cc @@ -54,14 +54,18 @@ class CompilerImpl : public Compiler { validator_(std::move(validator)), options_(options) {} - absl::StatusOr Compile( - absl::string_view expression, absl::string_view description, - google::protobuf::Arena* arena) const override { - CEL_ASSIGN_OR_RETURN(auto source, - cel::NewSource(expression, std::string(description))); + std::unique_ptr ToBuilder() const override; + + const TypeChecker& GetTypeChecker() const override { return *type_checker_; } + const Parser& GetParser() const override { return *parser_; } + const Validator& GetValidator() const override { return validator_; } + + protected: + absl::StatusOr CompileImpl( + const Source& source, google::protobuf::Arena* arena) const override { std::vector parse_issues; absl::StatusOr> ast = - parser_->Parse(*source, &parse_issues); + parser_->Parse(source, &parse_issues); if (!ast.ok()) { if (!options_.adapt_parser_errors || ast.status().code() != absl::StatusCode::kInvalidArgument || @@ -74,26 +78,17 @@ class CompilerImpl : public Compiler { check_issues.push_back(TypeCheckIssue::CreateError( issue.location(), std::string(issue.message()))); } - ValidationResult result(std::move(check_issues)); - result.SetSource(std::move(source)); - return result; + return ValidationResult(std::move(check_issues)); } CEL_ASSIGN_OR_RETURN(ValidationResult result, type_checker_->Check(*std::move(ast), arena)); - result.SetSource(std::move(source)); if (!validator_.validations().empty()) { validator_.UpdateValidationResult(result); } return result; } - std::unique_ptr ToBuilder() const override; - - const TypeChecker& GetTypeChecker() const override { return *type_checker_; } - const Parser& GetParser() const override { return *parser_; } - const Validator& GetValidator() const override { return validator_; } - private: std::unique_ptr type_checker_; std::unique_ptr parser_; diff --git a/compiler/compiler_factory_test.cc b/compiler/compiler_factory_test.cc index 035fd8aa6..3f83f5bf2 100644 --- a/compiler/compiler_factory_test.cc +++ b/compiler/compiler_factory_test.cc @@ -427,5 +427,19 @@ TEST(CompilerFactoryTest, ReturnsIssuesFromParser) { EXPECT_THAT(result.GetIssues(), testing::Not(testing::IsEmpty())); } +TEST(CompilerFactoryTest, CompileSourceOverload) { + ASSERT_OK_AND_ASSIGN( + auto builder, + NewCompilerBuilder(cel::internal::GetSharedTestingDescriptorPool())); + + ASSERT_THAT(builder->AddLibrary(StandardCompilerLibrary()), IsOk()); + ASSERT_OK_AND_ASSIGN(auto compiler, builder->Build()); + + ASSERT_OK_AND_ASSIGN(auto source, cel::NewSource("1 + 2")); + ASSERT_OK_AND_ASSIGN(ValidationResult result, compiler->Compile(*source)); + + EXPECT_TRUE(result.IsValid()); +} + } // namespace } // namespace cel