diff --git a/clang-tools-extra/clang-tidy/utils/TransformerClangTidyCheck.cpp b/clang-tools-extra/clang-tidy/utils/TransformerClangTidyCheck.cpp index 515997b14231..821c6290fe01 100644 --- a/clang-tools-extra/clang-tidy/utils/TransformerClangTidyCheck.cpp +++ b/clang-tools-extra/clang-tidy/utils/TransformerClangTidyCheck.cpp @@ -73,16 +73,15 @@ void TransformerClangTidyCheck::check( assert(Rule && "check() should not fire if Rule is None"); RewriteRule::Case Case = transformer::detail::findSelectedCase(Result, *Rule); - Expected> - Transformations = transformer::detail::translateEdits(Result, Case.Edits); - if (!Transformations) { - llvm::errs() << "Rewrite failed: " - << llvm::toString(Transformations.takeError()) << "\n"; + Expected> Edits = Case.Edits(Result); + if (!Edits) { + llvm::errs() << "Rewrite failed: " << llvm::toString(Edits.takeError()) + << "\n"; return; } // No rewrite applied, but no error encountered either. - if (Transformations->empty()) + if (Edits->empty()) return; Expected Explanation = Case.Explanation->eval(Result); @@ -93,9 +92,8 @@ void TransformerClangTidyCheck::check( } // Associate the diagnostic with the location of the first change. - DiagnosticBuilder Diag = - diag((*Transformations)[0].Range.getBegin(), *Explanation); - for (const auto &T : *Transformations) + DiagnosticBuilder Diag = diag((*Edits)[0].Range.getBegin(), *Explanation); + for (const auto &T : *Edits) Diag << FixItHint::CreateReplacement(T.Range, T.Replacement); for (const auto &I : Case.AddedIncludes) { diff --git a/clang/include/clang/Tooling/Transformer/RewriteRule.h b/clang/include/clang/Tooling/Transformer/RewriteRule.h index 4fca3bc9e2e8..4a5e5556cdff 100644 --- a/clang/include/clang/Tooling/Transformer/RewriteRule.h +++ b/clang/include/clang/Tooling/Transformer/RewriteRule.h @@ -30,6 +30,19 @@ namespace clang { namespace transformer { +/// A concrete description of a source edit, represented by a character range in +/// the source to be replaced and a corresponding replacement string. +struct Edit { + CharSourceRange Range; + std::string Replacement; +}; + +/// Maps a match result to a list of concrete edits (with possible +/// failure). This type is a building block of rewrite rules, but users will +/// generally work in terms of `ASTEdit`s (below) rather than directly in terms +/// of `EditGenerator`. +using EditGenerator = MatchConsumer>; + using TextGenerator = std::shared_ptr>; // Description of a source-code edit, expressed in terms of an AST node. @@ -74,6 +87,19 @@ struct ASTEdit { TextGenerator Note; }; +/// Lifts a list of `ASTEdit`s into an `EditGenerator`. +/// +/// The `EditGenerator` will return an empty vector if any of the edits apply to +/// portions of the source that are ineligible for rewriting (certain +/// interactions with macros, for example) and it will fail if any invariants +/// are violated relating to bound nodes in the match. However, it does not +/// fail in the case of conflicting edits -- conflict handling is left to +/// clients. We recommend use of the \c AtomicChange or \c Replacements classes +/// for assistance in detecting such conflicts. +EditGenerator editList(llvm::SmallVector Edits); +// Convenience form of `editList` for a single edit. +EditGenerator edit(ASTEdit); + /// Format of the path in an include directive -- angle brackets or quotes. enum class IncludeFormat { Quoted, @@ -106,7 +132,7 @@ enum class IncludeFormat { struct RewriteRule { struct Case { ast_matchers::internal::DynTypedMatcher Matcher; - SmallVector Edits; + EditGenerator Edits; TextGenerator Explanation; // Include paths to add to the file affected by this case. These are // bundled with the `Case`, rather than the `RewriteRule`, because each case @@ -123,16 +149,22 @@ struct RewriteRule { /// Convenience function for constructing a simple \c RewriteRule. RewriteRule makeRule(ast_matchers::internal::DynTypedMatcher M, - SmallVector Edits, - TextGenerator Explanation = nullptr); + EditGenerator Edits, TextGenerator Explanation = nullptr); + +/// Convenience function for constructing a \c RewriteRule from multiple +/// `ASTEdit`s. +inline RewriteRule makeRule(ast_matchers::internal::DynTypedMatcher M, + llvm::SmallVector Edits, + TextGenerator Explanation = nullptr) { + return makeRule(std::move(M), editList(std::move(Edits)), + std::move(Explanation)); +} /// Convenience overload of \c makeRule for common case of only one edit. inline RewriteRule makeRule(ast_matchers::internal::DynTypedMatcher M, ASTEdit Edit, TextGenerator Explanation = nullptr) { - SmallVector Edits; - Edits.emplace_back(std::move(Edit)); - return makeRule(std::move(M), std::move(Edits), std::move(Explanation)); + return makeRule(std::move(M), edit(std::move(Edit)), std::move(Explanation)); } /// For every case in Rule, adds an include directive for the given header. The @@ -260,28 +292,6 @@ getRuleMatchLoc(const ast_matchers::MatchFinder::MatchResult &Result); const RewriteRule::Case & findSelectedCase(const ast_matchers::MatchFinder::MatchResult &Result, const RewriteRule &Rule); - -/// A source "transformation," represented by a character range in the source to -/// be replaced and a corresponding replacement string. -struct Transformation { - CharSourceRange Range; - std::string Replacement; -}; - -/// Attempts to translate `Edits`, which are in terms of AST nodes bound in the -/// match `Result`, into Transformations, which are in terms of the source code -/// text. -/// -/// Returns an empty vector if any of the edits apply to portions of the source -/// that are ineligible for rewriting (certain interactions with macros, for -/// example). Fails if any invariants are violated relating to bound nodes in -/// the match. However, it does not fail in the case of conflicting edits -- -/// conflict handling is left to clients. We recommend use of the \c -/// AtomicChange or \c Replacements classes for assistance in detecting such -/// conflicts. -Expected> -translateEdits(const ast_matchers::MatchFinder::MatchResult &Result, - llvm::ArrayRef Edits); } // namespace detail } // namespace transformer diff --git a/clang/lib/Tooling/Transformer/RewriteRule.cpp b/clang/lib/Tooling/Transformer/RewriteRule.cpp index 599f20b18eb6..968f7fa6cd32 100644 --- a/clang/lib/Tooling/Transformer/RewriteRule.cpp +++ b/clang/lib/Tooling/Transformer/RewriteRule.cpp @@ -28,12 +28,11 @@ using ast_matchers::internal::DynTypedMatcher; using MatchResult = MatchFinder::MatchResult; -Expected> -transformer::detail::translateEdits(const MatchResult &Result, - llvm::ArrayRef Edits) { - SmallVector Transformations; - for (const auto &Edit : Edits) { - Expected Range = Edit.TargetRange(Result); +static Expected> +translateEdits(const MatchResult &Result, ArrayRef ASTEdits) { + SmallVector Edits; + for (const auto &E : ASTEdits) { + Expected Range = E.TargetRange(Result); if (!Range) return Range.takeError(); llvm::Optional EditRange = @@ -41,21 +40,33 @@ transformer::detail::translateEdits(const MatchResult &Result, // FIXME: let user specify whether to treat this case as an error or ignore // it as is currently done. if (!EditRange) - return SmallVector(); - auto Replacement = Edit.Replacement->eval(Result); + return SmallVector(); + auto Replacement = E.Replacement->eval(Result); if (!Replacement) return Replacement.takeError(); - transformer::detail::Transformation T; + transformer::Edit T; T.Range = *EditRange; T.Replacement = std::move(*Replacement); - Transformations.push_back(std::move(T)); + Edits.push_back(std::move(T)); } - return Transformations; + return Edits; } -ASTEdit transformer::changeTo(RangeSelector S, TextGenerator Replacement) { +EditGenerator transformer::editList(SmallVector Edits) { + return [Edits = std::move(Edits)](const MatchResult &Result) { + return translateEdits(Result, Edits); + }; +} + +EditGenerator transformer::edit(ASTEdit Edit) { + return [Edit = std::move(Edit)](const MatchResult &Result) { + return translateEdits(Result, {Edit}); + }; +} + +ASTEdit transformer::changeTo(RangeSelector Target, TextGenerator Replacement) { ASTEdit E; - E.TargetRange = std::move(S); + E.TargetRange = std::move(Target); E.Replacement = std::move(Replacement); return E; } @@ -82,8 +93,9 @@ ASTEdit transformer::remove(RangeSelector S) { return change(std::move(S), std::make_shared("")); } -RewriteRule transformer::makeRule(DynTypedMatcher M, SmallVector Edits, - TextGenerator Explanation) { +RewriteRule transformer::makeRule(ast_matchers::internal::DynTypedMatcher M, + EditGenerator Edits, + TextGenerator Explanation) { return RewriteRule{{RewriteRule::Case{ std::move(M), std::move(Edits), std::move(Explanation), {}}}}; } diff --git a/clang/lib/Tooling/Transformer/Transformer.cpp b/clang/lib/Tooling/Transformer/Transformer.cpp index 71f0646f4c0e..93c2c0912d21 100644 --- a/clang/lib/Tooling/Transformer/Transformer.cpp +++ b/clang/lib/Tooling/Transformer/Transformer.cpp @@ -31,7 +31,7 @@ void Transformer::run(const MatchFinder::MatchResult &Result) { transformer::RewriteRule::Case Case = transformer::detail::findSelectedCase(Result, Rule); - auto Transformations = transformer::detail::translateEdits(Result, Case.Edits); + auto Transformations = Case.Edits(Result); if (!Transformations) { Consumer(Transformations.takeError()); return;