Skip to content

Commit 2d6f3ed

Browse files
aibaarsaibaars
committed
Address comments
1 parent 858aa9a commit 2d6f3ed

File tree

3 files changed

+42
-35
lines changed

3 files changed

+42
-35
lines changed

‎ruby/extractor/src/diagnostics.rs

Lines changed: 15 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -214,10 +214,12 @@ fn longest_backtick_sequence_length(text: &str) -> usize {
214214
}
215215
result
216216
}
217-
pub enum Arg<'a> {
217+
/**
218+
* An argument of a diagnostic message format string. A message argument is either a "code" snippet or a link.
219+
*/
220+
pub enum MessageArg<'a> {
218221
Code(&'a str),
219222
Link(&'a str, &'a str),
220-
None,
221223
}
222224

223225
impl DiagnosticMessage {
@@ -242,16 +244,15 @@ impl DiagnosticMessage {
242244
self
243245
}
244246

245-
pub fn message(&mut self, text: &str, args: &[Arg]) -> &mut Self {
247+
pub fn message(&mut self, text: &str, args: &[MessageArg]) -> &mut Self {
246248
let parts = text.split("{}");
247-
let args = args.iter().chain(std::iter::repeat(&Arg::None));
248249
let mut plain = String::with_capacity(2 * text.len());
249250
let mut markdown = String::with_capacity(2 * text.len());
250-
for (p, a) in parts.zip(args) {
251+
for (i, p) in parts.enumerate() {
251252
plain.push_str(p);
252253
markdown.push_str(p);
253-
match a {
254-
Arg::Code(t) => {
254+
match args.get(i) {
255+
Some(MessageArg::Code(t)) => {
255256
plain.push_str(t);
256257
if t.len() > 0 {
257258
let count = longest_backtick_sequence_length(t) + 1;
@@ -266,7 +267,7 @@ impl DiagnosticMessage {
266267
markdown.push_str(&"`".repeat(count));
267268
}
268269
}
269-
Arg::Link(text, url) => {
270+
Some(MessageArg::Link(text, url)) => {
270271
plain.push_str(text);
271272
self.help_link(url);
272273
markdown.push_str("[");
@@ -275,7 +276,7 @@ impl DiagnosticMessage {
275276
markdown.push_str(url);
276277
markdown.push_str(")");
277278
}
278-
Arg::None => {}
279+
None => {}
279280
}
280281
}
281282
self.text(&plain);
@@ -343,14 +344,17 @@ fn test_message() {
343344
let mut m = DiagnosticLoggers::new("foo")
344345
.logger()
345346
.new_entry("id", "name");
346-
m.message("hello: {}", &[Arg::Code("hello")]);
347+
m.message("hello: {}", &[MessageArg::Code("hello")]);
347348
assert_eq!("hello: hello", m.plaintext_message);
348349
assert_eq!("hello: `hello`", m.markdown_message);
349350

350351
let mut m = DiagnosticLoggers::new("foo")
351352
.logger()
352353
.new_entry("id", "name");
353-
m.message("hello with backticks: {}", &[Arg::Code("oh `hello`!")]);
354+
m.message(
355+
"hello with backticks: {}",
356+
&[MessageArg::Code("oh `hello`!")],
357+
);
354358
assert_eq!("hello with backticks: oh `hello`!", m.plaintext_message);
355359
assert_eq!(
356360
"hello with backticks: `` oh `hello`! ``",

‎ruby/extractor/src/extractor.rs

Lines changed: 15 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -277,7 +277,7 @@ impl<'a> Visitor<'a> {
277277
fn record_parse_error_for_node(
278278
&mut self,
279279
message: &str,
280-
args: &[diagnostics::Arg],
280+
args: &[diagnostics::MessageArg],
281281
node: Node,
282282
status_page: bool,
283283
) {
@@ -307,7 +307,7 @@ impl<'a> Visitor<'a> {
307307
if node.is_missing() {
308308
self.record_parse_error_for_node(
309309
"A parse error occurred (expected {} symbol). Check the syntax of the file. If the file is invalid, correct the error or {} the file from analysis.",
310-
&[diagnostics::Arg::Code(node.kind()), diagnostics::Arg::Link("exclude", "https://docs.github.com/en/code-security/code-scanning/automatically-scanning-your-code-for-vulnerabilities-and-errors/customizing-code-scanning")],
310+
&[diagnostics::MessageArg::Code(node.kind()), diagnostics::MessageArg::Link("exclude", "https://docs.github.com/en/code-security/code-scanning/automatically-scanning-your-code-for-vulnerabilities-and-errors/customizing-code-scanning")],
311311
node,
312312
true,
313313
);
@@ -316,7 +316,7 @@ impl<'a> Visitor<'a> {
316316
if node.is_error() {
317317
self.record_parse_error_for_node(
318318
"A parse error occurred. Check the syntax of the file. If the file is invalid, correct the error or {} the file from analysis.",
319-
&[diagnostics::Arg::Link("exclude", "https://docs.github.com/en/code-security/code-scanning/automatically-scanning-your-code-for-vulnerabilities-and-errors/customizing-code-scanning")],
319+
&[diagnostics::MessageArg::Link("exclude", "https://docs.github.com/en/code-security/code-scanning/automatically-scanning-your-code-for-vulnerabilities-and-errors/customizing-code-scanning")],
320320
node,
321321
true,
322322
);
@@ -409,7 +409,7 @@ impl<'a> Visitor<'a> {
409409
.location(self.path, start_line, start_column, end_line, end_column)
410410
.message(
411411
"Unknown table type: {}",
412-
&[diagnostics::Arg::Code(node.kind())],
412+
&[diagnostics::MessageArg::Code(node.kind())],
413413
),
414414
);
415415

@@ -461,10 +461,10 @@ impl<'a> Visitor<'a> {
461461
self.record_parse_error_for_node(
462462
"Type mismatch for field {}::{} with type {} != {}",
463463
&[
464-
diagnostics::Arg::Code(node.kind()),
465-
diagnostics::Arg::Code(child_node.field_name.unwrap_or("child")),
466-
diagnostics::Arg::Code(&format!("{:?}", child_node.type_name)),
467-
diagnostics::Arg::Code(&format!("{:?}", field.type_info)),
464+
diagnostics::MessageArg::Code(node.kind()),
465+
diagnostics::MessageArg::Code(child_node.field_name.unwrap_or("child")),
466+
diagnostics::MessageArg::Code(&format!("{:?}", child_node.type_name)),
467+
diagnostics::MessageArg::Code(&format!("{:?}", field.type_info)),
468468
],
469469
*node,
470470
false,
@@ -474,9 +474,9 @@ impl<'a> Visitor<'a> {
474474
self.record_parse_error_for_node(
475475
"Value for unknown field: {}::{} and type {}",
476476
&[
477-
diagnostics::Arg::Code(node.kind()),
478-
diagnostics::Arg::Code(&child_node.field_name.unwrap_or("child")),
479-
diagnostics::Arg::Code(&format!("{:?}", child_node.type_name)),
477+
diagnostics::MessageArg::Code(node.kind()),
478+
diagnostics::MessageArg::Code(&child_node.field_name.unwrap_or("child")),
479+
diagnostics::MessageArg::Code(&format!("{:?}", child_node.type_name)),
480480
],
481481
*node,
482482
false,
@@ -516,8 +516,8 @@ impl<'a> Visitor<'a> {
516516
self.record_parse_error_for_node(
517517
"Too many values for field: {}::{}",
518518
&[
519-
diagnostics::Arg::Code(node.kind()),
520-
diagnostics::Arg::Code(table_name),
519+
diagnostics::MessageArg::Code(node.kind()),
520+
diagnostics::MessageArg::Code(table_name),
521521
],
522522
*node,
523523
false,
@@ -637,8 +637,8 @@ fn location_for(visitor: &mut Visitor, n: Node) -> (usize, usize, usize, usize)
637637
.message(
638638
"Cannot correct end column value: end_byte index {} is not in range [1,{}].",
639639
&[
640-
diagnostics::Arg::Code(&index.to_string()),
641-
diagnostics::Arg::Code(&source.len().to_string()),
640+
diagnostics::MessageArg::Code(&index.to_string()),
641+
diagnostics::MessageArg::Code(&source.len().to_string()),
642642
],
643643
)
644644
.severity(diagnostics::Severity::Error),

‎ruby/extractor/src/main.rs

Lines changed: 12 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -74,7 +74,10 @@ fn main() -> std::io::Result<()> {
7474
main_thread_logger.write(
7575
main_thread_logger
7676
.new_entry("configuration-error", "Configuration error")
77-
.message("{}; defaulting to 1 thread.", &[diagnostics::Arg::Code(&e)])
77+
.message(
78+
"{}; defaulting to 1 thread.",
79+
&[diagnostics::MessageArg::Code(&e)],
80+
)
7881
.severity(diagnostics::Severity::Warning),
7982
);
8083
1
@@ -95,7 +98,7 @@ fn main() -> std::io::Result<()> {
9598
main_thread_logger.write(
9699
main_thread_logger
97100
.new_entry("configuration-error", "Configuration error")
98-
.message("{}; using gzip.", &[diagnostics::Arg::Code(&e)])
101+
.message("{}; using gzip.", &[diagnostics::MessageArg::Code(&e)])
99102
.severity(diagnostics::Severity::Warning),
100103
);
101104
trap::Compression::Gzip
@@ -205,10 +208,10 @@ fn main() -> std::io::Result<()> {
205208
.message(
206209
"Could not decode the file contents as {}: {}. The contents of the file must match the character encoding specified in the {} {}.",
207210
&[
208-
diagnostics::Arg::Code(&encoding_name),
209-
diagnostics::Arg::Code(&msg),
210-
diagnostics::Arg::Code("encoding:"),
211-
diagnostics::Arg::Link("directive", "https://docs.ruby-lang.org/en/master/syntax/comments_rdoc.html#label-encoding+Directive")
211+
diagnostics::MessageArg::Code(&encoding_name),
212+
diagnostics::MessageArg::Code(&msg),
213+
diagnostics::MessageArg::Code("encoding:"),
214+
diagnostics::MessageArg::Link("directive", "https://docs.ruby-lang.org/en/master/syntax/comments_rdoc.html#label-encoding+Directive")
212215
],
213216
)
214217
.status_page()
@@ -225,9 +228,9 @@ fn main() -> std::io::Result<()> {
225228
.message(
226229
"Unknown character encoding {} in {} {}.",
227230
&[
228-
diagnostics::Arg::Code(&encoding_name),
229-
diagnostics::Arg::Code("#encoding:"),
230-
diagnostics::Arg::Link("directive", "https://docs.ruby-lang.org/en/master/syntax/comments_rdoc.html#label-encoding+Directive")
231+
diagnostics::MessageArg::Code(&encoding_name),
232+
diagnostics::MessageArg::Code("#encoding:"),
233+
diagnostics::MessageArg::Link("directive", "https://docs.ruby-lang.org/en/master/syntax/comments_rdoc.html#label-encoding+Directive")
231234
],
232235
)
233236
.status_page()

0 commit comments

Comments
 (0)