Skip to content

Commit

Permalink
impl(generator): better blockquote handling (#346)
Browse files Browse the repository at this point in the history
Rust treats any blockquote as a Rust code sample, unless it has the
`norust` tag. Furthermore, `cargo test` will try to compile these code
samples. This is all very good, but we generate the documentation
comments from the Probobuf (or OpenAPI) comments, which may and do
include blockquotes that are not Rust code.

This change handles a few more blockquote types. We really need to parse
the Markdown comments to handle all possible cases, but this can handle
the most urgent cases.
  • Loading branch information
coryan authored Dec 2, 2024
1 parent 82261da commit da6f32b
Show file tree
Hide file tree
Showing 12 changed files with 595 additions and 329 deletions.
70 changes: 59 additions & 11 deletions generator/internal/language/rust.go
Original file line number Diff line number Diff line change
Expand Up @@ -609,21 +609,69 @@ func (*RustCodec) ToCamel(symbol string) string {
return rustEscapeKeyword(strcase.ToLowerCamel(symbol))
}

// TODO(#92) - protect all quotes with `norust`
// TODO(#30) - convert protobuf links to Rusty links.
//
// Blockquotes require special treatment for Rust. By default, Rust assumes
// blockquotes contain compilable Rust code samples. To override the default
// the blockquote must be marked with "```norust". The proto comments have
// many blockquotes that do not follow this convention (nor should they).
//
// This function handles some easy cases of blockquotes, but a full treatment
// requires parsing of the comments. The CommonMark [spec] includes some
// difficult examples.
//
// [spec]: https://spec.commonmark.org/0.13/#block-quotes
func (*RustCodec) FormatDocComments(documentation string) []string {
inBlockQuote := false
ss := strings.Split(documentation, "\n")
for i := range ss {
if strings.HasSuffix(ss[i], "```") {
if !inBlockQuote {
ss[i] = ss[i] + "norust"
inBlockquote := false
blockquotePrefix := ""

var results []string
for _, line := range strings.Split(documentation, "\n") {
if inBlockquote {
switch {
case line == "```":
inBlockquote = false
results = append(results, "```")
case strings.HasPrefix(line, blockquotePrefix):
results = append(results, strings.TrimPrefix(line, blockquotePrefix))
default:
inBlockquote = false
results = append(results, "```")
results = append(results, line)
}
} else {
switch {
case line == "```":
results = append(results, "```norust")
inBlockquote = true
case strings.HasPrefix(line, " "):
inBlockquote = true
blockquotePrefix = " "
results = append(results, "```norust")
results = append(results, strings.TrimPrefix(line, blockquotePrefix))
case strings.HasPrefix(line, " > "):
inBlockquote = true
blockquotePrefix = " > "
results = append(results, "```norust")
results = append(results, strings.TrimPrefix(line, blockquotePrefix))
case strings.HasPrefix(line, "> "):
inBlockquote = true
blockquotePrefix = "> "
results = append(results, "```norust")
results = append(results, strings.TrimPrefix(line, blockquotePrefix))
default:
results = append(results, line)
}
inBlockQuote = !inBlockQuote
}
ss[i] = fmt.Sprintf("/// %s", ss[i])
// nit: remove the trailing whitespace, this is unsightly.
ss[i] = strings.TrimRightFunc(ss[i], unicode.IsSpace)
}
return ss
if inBlockquote {
results = append(results, "```")
}
for i, line := range results {
results[i] = strings.TrimRightFunc(fmt.Sprintf("/// %s", line), unicode.IsSpace)
}
return results
}

func (c *RustCodec) projectRoot() string {
Expand Down
102 changes: 102 additions & 0 deletions generator/internal/language/rust_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -939,6 +939,108 @@ func TestRust_FormatDocCommentsBullets(t *testing.T) {
}
}

func TestRust_FormatDocCommentsImplicitBlockQuote(t *testing.T) {
input := `
Blockquotes come in many forms. They can start with a leading '> ', as in:
> Block quote style 1
> Continues 1 - style 1
> Continues 2 - style 1
> Continues 3 - style 1
They can start with 3 spaces and then '> ', as in:
> Block quote style 2
> Continues 1 - style 2
> Continues 2 - style 2
> Continues 3 - style 2
Or they can start with just 4 spaces:
Block quote style 3
Continues 1 - style 3
Continues 2 - style 3
Continues 3 - style 3
Note that four spaces and a leading '> ' makes the '> ' prefix part of the
block:
> Block quote with arrow.
> Continues 1 - with arrow
> Continues 2 - with arrow
Continues 3 - with arrow
`

want := []string{
"///",
"/// Blockquotes come in many forms. They can start with a leading '> ', as in:",
"///",
"/// ```norust",
"/// Block quote style 1",
"/// Continues 1 - style 1",
"/// Continues 2 - style 1",
"/// Continues 3 - style 1",
"/// ```",
"///",
"/// They can start with 3 spaces and then '> ', as in:",
"///",
"/// ```norust",
"/// Block quote style 2",
"/// Continues 1 - style 2",
"/// Continues 2 - style 2",
"/// Continues 3 - style 2",
"/// ```",
"///",
"/// Or they can start with just 4 spaces:",
"///",
"/// ```norust",
"/// Block quote style 3",
"/// Continues 1 - style 3",
"/// Continues 2 - style 3",
"/// Continues 3 - style 3",
"/// ```",
"///",
"/// Note that four spaces and a leading '> ' makes the '> ' prefix part of the",
"/// block:",
"///",
"/// ```norust",
"/// > Block quote with arrow.",
"/// > Continues 1 - with arrow",
"/// > Continues 2 - with arrow",
"/// Continues 3 - with arrow",
"/// ```",
"///",
"///",
}

c := &RustCodec{}
got := c.FormatDocComments(input)
if diff := cmp.Diff(want, got); diff != "" {
t.Errorf("mismatch in FormatDocComments (-want, +got)\n:%s", diff)
}
}

func TestRust_FormatDocCommentsImplicitBlockQuoteClosing(t *testing.T) {
input := `Blockquotes can appear at the end of the comment:
they should have a closing element.`

want := []string{
"/// Blockquotes can appear at the end of the comment:",
"///",
"/// ```norust",
"/// they should have a closing element.",
"/// ```",
}

c := &RustCodec{}
got := c.FormatDocComments(input)
if diff := cmp.Diff(want, got); diff != "" {
t.Errorf("mismatch in FormatDocComments (-want, +got)\n:%s", diff)
}
}

func TestRust_MessageNames(t *testing.T) {
message := &api.Message{
Name: "Replication",
Expand Down
76 changes: 40 additions & 36 deletions generator/testdata/rust/gclient/golden/iam/v1/src/model.rs
Original file line number Diff line number Diff line change
Expand Up @@ -437,41 +437,43 @@ impl Binding {
///
/// Example Policy with multiple AuditConfigs:
///
/// ```norust
/// {
/// "audit_configs": [
/// {
/// "audit_configs": [
/// "service": "allServices",
/// "audit_log_configs": [
/// {
/// "service": "allServices",
/// "audit_log_configs": [
/// {
/// "log_type": "DATA_READ",
/// "exempted_members": [
/// "user:[email protected]"
/// ]
/// },
/// {
/// "log_type": "DATA_WRITE"
/// },
/// {
/// "log_type": "ADMIN_READ"
/// }
/// "log_type": "DATA_READ",
/// "exempted_members": [
/// "user:[email protected]"
/// ]
/// },
/// {
/// "service": "sampleservice.googleapis.com",
/// "audit_log_configs": [
/// {
/// "log_type": "DATA_READ"
/// },
/// {
/// "log_type": "DATA_WRITE",
/// "exempted_members": [
/// "user:[email protected]"
/// ]
/// }
/// "log_type": "DATA_WRITE"
/// },
/// {
/// "log_type": "ADMIN_READ"
/// }
/// ]
/// },
/// {
/// "service": "sampleservice.googleapis.com",
/// "audit_log_configs": [
/// {
/// "log_type": "DATA_READ"
/// },
/// {
/// "log_type": "DATA_WRITE",
/// "exempted_members": [
/// "user:[email protected]"
/// ]
/// }
/// ]
/// }
/// ]
/// }
/// ```
///
/// For sampleservice, this policy enables DATA_READ, DATA_WRITE and ADMIN_READ
/// logging. It also exempts `[email protected]` from DATA_READ logging, and
Expand Down Expand Up @@ -510,19 +512,21 @@ impl AuditConfig {
/// Provides the configuration for logging a type of permissions.
/// Example:
///
/// ```norust
/// {
/// "audit_log_configs": [
/// {
/// "audit_log_configs": [
/// {
/// "log_type": "DATA_READ",
/// "exempted_members": [
/// "user:[email protected]"
/// ]
/// },
/// {
/// "log_type": "DATA_WRITE"
/// }
/// "log_type": "DATA_READ",
/// "exempted_members": [
/// "user:[email protected]"
/// ]
/// },
/// {
/// "log_type": "DATA_WRITE"
/// }
/// ]
/// }
/// ```
///
/// This enables 'DATA_READ' and 'DATA_WRITE' logging, while exempting
/// [email protected] from DATA_READ logging.
Expand Down
4 changes: 3 additions & 1 deletion generator/testdata/rust/gclient/golden/location/src/model.rs
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,9 @@ pub struct Location {

/// Cross-service attributes for the location. For example
///
/// {"cloud.googleapis.com/region": "us-east1"}
/// ```norust
/// {"cloud.googleapis.com/region": "us-east1"}
/// ```
#[serde(skip_serializing_if = "std::collections::HashMap::is_empty")]
pub labels: std::collections::HashMap<String, String>,

Expand Down
62 changes: 36 additions & 26 deletions generator/testdata/rust/gclient/golden/module/rpc/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,25 +19,29 @@
/// Example of an error when contacting the "pubsub.googleapis.com" API when it
/// is not enabled:
///
/// { "reason": "API_DISABLED"
/// "domain": "googleapis.com"
/// "metadata": {
/// "resource": "projects/123",
/// "service": "pubsub.googleapis.com"
/// }
/// }
/// ```norust
/// { "reason": "API_DISABLED"
/// "domain": "googleapis.com"
/// "metadata": {
/// "resource": "projects/123",
/// "service": "pubsub.googleapis.com"
/// }
/// }
/// ```
///
/// This response indicates that the pubsub.googleapis.com API is not enabled.
///
/// Example of an error that is returned when attempting to create a Spanner
/// instance in a region that is out of stock:
///
/// { "reason": "STOCKOUT"
/// "domain": "spanner.googleapis.com",
/// "metadata": {
/// "availableRegions": "us-central1,us-east2"
/// }
/// }
/// ```norust
/// { "reason": "STOCKOUT"
/// "domain": "spanner.googleapis.com",
/// "metadata": {
/// "availableRegions": "us-central1,us-east2"
/// }
/// }
/// ```
#[serde_with::serde_as]
#[derive(Clone, Debug, Default, PartialEq, serde::Deserialize, serde::Serialize)]
#[serde(rename_all = "camelCase")]
Expand Down Expand Up @@ -336,21 +340,27 @@ pub mod bad_request {
///
/// Consider the following:
///
/// message CreateContactRequest {
/// message EmailAddress {
/// enum Type {
/// TYPE_UNSPECIFIED = 0;
/// HOME = 1;
/// WORK = 2;
/// }
/// ```norust
/// message CreateContactRequest {
/// message EmailAddress {
/// enum Type {
/// TYPE_UNSPECIFIED = 0;
/// HOME = 1;
/// WORK = 2;
/// }
/// ```
///
/// optional string email = 1;
/// repeated EmailType type = 2;
/// }
/// ```norust
/// optional string email = 1;
/// repeated EmailType type = 2;
/// }
/// ```
///
/// string full_name = 1;
/// repeated EmailAddress email_addresses = 2;
/// }
/// ```norust
/// string full_name = 1;
/// repeated EmailAddress email_addresses = 2;
/// }
/// ```
///
/// In this example, in proto `field` could take one of the following values:
///
Expand Down
Loading

0 comments on commit da6f32b

Please sign in to comment.