From 1a63fe0f831ee8c15c43ebdc4b065dd1f5e0cbe5 Mon Sep 17 00:00:00 2001 From: Abeeujah Date: Thu, 31 Oct 2024 18:31:56 +0100 Subject: [PATCH 1/4] feat: Prevent text before the first heading --- eipw-lint/src/lints/markdown.rs | 2 + eipw-lint/src/lints/markdown/headings_only.rs | 34 +++++++ .../tests/lint_markdown_headings_only.rs | 90 +++++++++++++++++++ 3 files changed, 126 insertions(+) create mode 100644 eipw-lint/src/lints/markdown/headings_only.rs create mode 100644 eipw-lint/tests/lint_markdown_headings_only.rs diff --git a/eipw-lint/src/lints/markdown.rs b/eipw-lint/src/lints/markdown.rs index 264329e1..4e17e3e4 100644 --- a/eipw-lint/src/lints/markdown.rs +++ b/eipw-lint/src/lints/markdown.rs @@ -4,6 +4,7 @@ * file, You can obtain one at https://mozilla.org/MPL/2.0/. */ +pub mod headings_only; pub mod headings_space; pub mod html_comments; pub mod json_schema; @@ -16,6 +17,7 @@ pub mod relative_links; pub mod section_order; pub mod section_required; +pub use self::headings_only::HeadingsOnly; pub use self::headings_space::HeadingsSpace; pub use self::html_comments::HtmlComments; pub use self::json_schema::JsonSchema; diff --git a/eipw-lint/src/lints/markdown/headings_only.rs b/eipw-lint/src/lints/markdown/headings_only.rs new file mode 100644 index 00000000..66d176d4 --- /dev/null +++ b/eipw-lint/src/lints/markdown/headings_only.rs @@ -0,0 +1,34 @@ +/* + * This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, You can obtain one at https://mozilla.org/MPL/2.0/. + */ + +use comrak::nodes::NodeValue; +use eipw_snippets::Level; + +use crate::lints::{Error, Lint}; + +#[derive(Debug)] +pub struct HeadingsOnly; + +impl Lint for HeadingsOnly { + fn lint<'a>(&self, slug: &'a str, ctx: &crate::lints::Context<'a, '_>) -> Result<(), Error> { + let second = ctx + .body() + .descendants() + .nth(1) + .expect("cannot submit an empty proposal") + .data + .borrow() + .to_owned() + .value; + match second { + NodeValue::Heading(_) => Ok(()), + _ => { + let annotation_type = Level::Error; + ctx.report(annotation_type.title("Only Heading is allowed after FrontMatter")) + } + } + } +} diff --git a/eipw-lint/tests/lint_markdown_headings_only.rs b/eipw-lint/tests/lint_markdown_headings_only.rs new file mode 100644 index 00000000..02ad0b4e --- /dev/null +++ b/eipw-lint/tests/lint_markdown_headings_only.rs @@ -0,0 +1,90 @@ +/* + * This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, You can obtain one at https://mozilla.org/MPL/2.0/. + */ + +use eipw_lint::{lints::markdown::HeadingsOnly, reporters::Text, Linter}; + +#[tokio::test] +async fn invalid_eip() { + let src = r#"--- +eip: 1234 +--- + +This is some text that appears before the first heading. Authors sometimes try +to write an introduction or preface to their proposal here. We don't want to allow +this. + +## Abstract + +After the "Abstract" heading is the first place we want to allow text."#; + + let reports = Linter::>::default() + .clear_lints() + .deny("markdown-headings-only", HeadingsOnly {}) + .check_slice(None, src) + .run() + .await + .unwrap() + .into_inner(); + + assert_eq!( + reports.trim(), + "error: Only Heading is allowed after FrontMatter" + ); +} + +#[tokio::test] +async fn valid_eip() { + let src = r#"--- +eip: 100 +title: Change difficulty adjustment to target mean block time including uncles +author: Vitalik Buterin (@vbuterin) +type: Standards Track +category: Core +status: Final +created: 2016-04-28 +--- + +### Specification + +Currently, the formula to compute the difficulty of a block includes the following logic: + +``` python +adj_factor = max(1 - ((timestamp - parent.timestamp) // 10), -99) +child_diff = int(max(parent.difficulty + (parent.difficulty // BLOCK_DIFF_FACTOR) * adj_factor, min(parent.difficulty, MIN_DIFF))) +... +``` + +If `block.number >= BYZANTIUM_FORK_BLKNUM`, we change the first line to the following: + +``` python +adj_factor = max((2 if len(parent.uncles) else 1) - ((timestamp - parent.timestamp) // 9), -99) +``` +### Rationale + +This new formula ensures that the difficulty adjustment algorithm targets a constant average rate of blocks produced including uncles, and so ensures a highly predictable issuance rate that cannot be manipulated upward by manipulating the uncle rate. A formula that accounts for the exact number of included uncles: +``` python +adj_factor = max(1 + len(parent.uncles) - ((timestamp - parent.timestamp) // 9), -99) +``` +can be fairly easily seen to be (to within a tolerance of ~3/4194304) mathematically equivalent to assuming that a block with `k` uncles is equivalent to a sequence of `k+1` blocks that all appear with the exact same timestamp, and this is likely the simplest possible way to accomplish the desired effect. But since the exact formula depends on the full block and not just the header, we are instead using an approximate formula that accomplishes almost the same effect but has the benefit that it depends only on the block header (as you can check the uncle hash against the blank hash). + +Changing the denominator from 10 to 9 ensures that the block time remains roughly the same (in fact, it should decrease by ~3% given the current uncle rate of 7%). + +### References + +1. EIP 100 issue and discussion: https://github.com/ethereum/EIPs/issues/100 +2. https://bitslog.wordpress.com/2016/04/28/uncle-mining-an-ethereum-consensus-protocol-flaw/"#; + + let reports = Linter::>::default() + .clear_lints() + .deny("markdown-headings-only", HeadingsOnly {}) + .check_slice(None, src) + .run() + .await + .unwrap() + .into_inner(); + + assert_eq!(reports, ""); +} From 97da96df6fccbf2aefd0bee24a34787516e1d678 Mon Sep 17 00:00:00 2001 From: Abeeujah Date: Sat, 2 Nov 2024 13:02:35 +0100 Subject: [PATCH 2/4] Error handling to improve function ergonomics --- eipw-lint/src/lints/markdown/headings_only.rs | 14 +++++--------- 1 file changed, 5 insertions(+), 9 deletions(-) diff --git a/eipw-lint/src/lints/markdown/headings_only.rs b/eipw-lint/src/lints/markdown/headings_only.rs index 66d176d4..b891a28b 100644 --- a/eipw-lint/src/lints/markdown/headings_only.rs +++ b/eipw-lint/src/lints/markdown/headings_only.rs @@ -14,15 +14,11 @@ pub struct HeadingsOnly; impl Lint for HeadingsOnly { fn lint<'a>(&self, slug: &'a str, ctx: &crate::lints::Context<'a, '_>) -> Result<(), Error> { - let second = ctx - .body() - .descendants() - .nth(1) - .expect("cannot submit an empty proposal") - .data - .borrow() - .to_owned() - .value; + let annotation_type = Level::Error; + let second = match ctx.body().descendants().nth(1) { + Some(el) => el.data.borrow().to_owned().value, + None => return ctx.report(annotation_type.title("Cannot submit an empty proposal")), + }; match second { NodeValue::Heading(_) => Ok(()), _ => { From 1e8970398f245795c719fa260509c6fefeaa9e86 Mon Sep 17 00:00:00 2001 From: Abeeujah Date: Mon, 4 Nov 2024 16:08:11 +0100 Subject: [PATCH 3/4] refac: Implemented reviewed changes to maintain consistency and improve error message ergonomics. --- eipw-lint/src/lints/markdown.rs | 4 ++-- .../{headings_only.rs => heading_first.rs} | 23 +++++++++++-------- ...only.rs => lint_markdown_heading_first.rs} | 8 +++---- 3 files changed, 20 insertions(+), 15 deletions(-) rename eipw-lint/src/lints/markdown/{headings_only.rs => heading_first.rs} (56%) rename eipw-lint/tests/{lint_markdown_headings_only.rs => lint_markdown_heading_first.rs} (91%) diff --git a/eipw-lint/src/lints/markdown.rs b/eipw-lint/src/lints/markdown.rs index 4e17e3e4..41d07591 100644 --- a/eipw-lint/src/lints/markdown.rs +++ b/eipw-lint/src/lints/markdown.rs @@ -4,7 +4,7 @@ * file, You can obtain one at https://mozilla.org/MPL/2.0/. */ -pub mod headings_only; +pub mod heading_first; pub mod headings_space; pub mod html_comments; pub mod json_schema; @@ -17,7 +17,7 @@ pub mod relative_links; pub mod section_order; pub mod section_required; -pub use self::headings_only::HeadingsOnly; +pub use self::heading_first::HeadingFirst; pub use self::headings_space::HeadingsSpace; pub use self::html_comments::HtmlComments; pub use self::json_schema::JsonSchema; diff --git a/eipw-lint/src/lints/markdown/headings_only.rs b/eipw-lint/src/lints/markdown/heading_first.rs similarity index 56% rename from eipw-lint/src/lints/markdown/headings_only.rs rename to eipw-lint/src/lints/markdown/heading_first.rs index b891a28b..8e13d482 100644 --- a/eipw-lint/src/lints/markdown/headings_only.rs +++ b/eipw-lint/src/lints/markdown/heading_first.rs @@ -5,26 +5,31 @@ */ use comrak::nodes::NodeValue; -use eipw_snippets::Level; use crate::lints::{Error, Lint}; #[derive(Debug)] -pub struct HeadingsOnly; +pub struct HeadingFirst; -impl Lint for HeadingsOnly { +impl Lint for HeadingFirst { fn lint<'a>(&self, slug: &'a str, ctx: &crate::lints::Context<'a, '_>) -> Result<(), Error> { - let annotation_type = Level::Error; let second = match ctx.body().descendants().nth(1) { Some(el) => el.data.borrow().to_owned().value, - None => return ctx.report(annotation_type.title("Cannot submit an empty proposal")), + None => { + return ctx.report( + ctx.annotation_level() + .title("Cannot submit an empty proposal") + .id(slug), + ) + } }; match second { NodeValue::Heading(_) => Ok(()), - _ => { - let annotation_type = Level::Error; - ctx.report(annotation_type.title("Only Heading is allowed after FrontMatter")) - } + _ => ctx.report( + ctx.annotation_level() + .title("Nothing is permitted between the preamble and the first heading") + .id(slug), + ), } } } diff --git a/eipw-lint/tests/lint_markdown_headings_only.rs b/eipw-lint/tests/lint_markdown_heading_first.rs similarity index 91% rename from eipw-lint/tests/lint_markdown_headings_only.rs rename to eipw-lint/tests/lint_markdown_heading_first.rs index 02ad0b4e..5a030829 100644 --- a/eipw-lint/tests/lint_markdown_headings_only.rs +++ b/eipw-lint/tests/lint_markdown_heading_first.rs @@ -4,7 +4,7 @@ * file, You can obtain one at https://mozilla.org/MPL/2.0/. */ -use eipw_lint::{lints::markdown::HeadingsOnly, reporters::Text, Linter}; +use eipw_lint::{lints::markdown::HeadingFirst, reporters::Text, Linter}; #[tokio::test] async fn invalid_eip() { @@ -22,7 +22,7 @@ After the "Abstract" heading is the first place we want to allow text."#; let reports = Linter::>::default() .clear_lints() - .deny("markdown-headings-only", HeadingsOnly {}) + .deny("markdown-headings-only", HeadingFirst {}) .check_slice(None, src) .run() .await @@ -31,7 +31,7 @@ After the "Abstract" heading is the first place we want to allow text."#; assert_eq!( reports.trim(), - "error: Only Heading is allowed after FrontMatter" + "error[markdown-headings-only]: Nothing is permitted between the preamble and the first heading" ); } @@ -79,7 +79,7 @@ Changing the denominator from 10 to 9 ensures that the block time remains roughl let reports = Linter::>::default() .clear_lints() - .deny("markdown-headings-only", HeadingsOnly {}) + .deny("markdown-headings-only", HeadingFirst {}) .check_slice(None, src) .run() .await From 3a6108187bc0600823ce3eb99e2c1214c44d3604 Mon Sep 17 00:00:00 2001 From: Sam Wilson Date: Tue, 5 Nov 2024 11:29:56 -0500 Subject: [PATCH 4/4] Document markdown-heading-first; more error detail --- README.md | 3 +- docs/markdown-heading-first/index.html | 42 ++++++++++++++++ eipw-lint/src/lib.rs | 4 ++ eipw-lint/src/lints/known_lints.rs | 4 ++ eipw-lint/src/lints/markdown/heading_first.rs | 50 ++++++++++++------- .../tests/lint_markdown_heading_first.rs | 39 ++++----------- 6 files changed, 93 insertions(+), 49 deletions(-) create mode 100644 docs/markdown-heading-first/index.html diff --git a/README.md b/README.md index 24823932..0e89eda6 100644 --- a/README.md +++ b/README.md @@ -75,7 +75,8 @@ error[preamble-order]: preamble header `description` must come after `title` | `markdown-refs` | ERCs are referenced using ERC-X, while other proposals use EIP-X. | | `markdown-rel-links` | All URLs in the page are relative. | | `markdown-req-section` | Required sections are present in the body of the proposal. | -| `markdown-headings-space` | Headers have a space after the leading '#' characters | +| `markdown-heading-first` | No content appears between preamble and first heading. | +| `markdown-headings-space` | Headers have a space after the leading '#' characters. | | `preamble-author` | The author header is correctly formatted, and there is at least one GitHub user listed. | | `preamble-date-created` | The `created` header is a date. | | `preamble-date-last-call-deadline` | The `last-call-deadline` header is a date. | diff --git a/docs/markdown-heading-first/index.html b/docs/markdown-heading-first/index.html new file mode 100644 index 00000000..d0928567 --- /dev/null +++ b/docs/markdown-heading-first/index.html @@ -0,0 +1,42 @@ + + + + + markdown-heading-first + + + + +
+

markdown-heading-first

+

+ No content appears between preamble and first heading. +

+ +
+

Examples

+ + 
+error[markdown-heading-first]: Nothing is permitted between the preamble and the first heading
+  --> input.md
+   |
+12 | This proposal describes the introduction in clients of a controlled gas limit increase strategy to determine the gas limit of a spec...
+   |
+
+
+

Explanation

+ +

+ markdown-heading-first ensures that no content + appears before the first heading. +

+ +

+ It is improper form to put text/markdown outside of a section. + Such text cannot be referred to in a URL (eg. #Section-Title), + nor does it appear in the table of contents. +

+
+
+ + diff --git a/eipw-lint/src/lib.rs b/eipw-lint/src/lib.rs index 7e9a2784..8f445fab 100644 --- a/eipw-lint/src/lib.rs +++ b/eipw-lint/src/lib.rs @@ -471,6 +471,10 @@ pub fn default_lints_enum() -> impl Iterator { sections: markdown::SectionRequired, }, MarkdownHeadingsSpace(markdown::HeadingsSpace), + MarkdownHeadingFirst(markdown::HeadingFirst), } impl DefaultLint @@ -114,6 +115,7 @@ where Self::MarkdownSectionOrder { sections } => Box::new(sections), Self::MarkdownSectionRequired { sections } => Box::new(sections), Self::MarkdownHeadingsSpace(l) => Box::new(l), + Self::MarkdownHeadingFirst(l) => Box::new(l), } } } @@ -154,6 +156,7 @@ where Self::MarkdownSectionOrder { sections } => sections, Self::MarkdownSectionRequired { sections } => sections, Self::MarkdownHeadingsSpace(l) => l, + Self::MarkdownHeadingFirst(l) => l, } } } @@ -290,6 +293,7 @@ where sections: markdown::SectionRequired(sections.0.iter().map(AsRef::as_ref).collect()), }, Self::MarkdownHeadingsSpace(l) => DefaultLint::MarkdownHeadingsSpace(l.clone()), + Self::MarkdownHeadingFirst(l) => DefaultLint::MarkdownHeadingFirst(l.clone()), } } } diff --git a/eipw-lint/src/lints/markdown/heading_first.rs b/eipw-lint/src/lints/markdown/heading_first.rs index 8e13d482..52adcc94 100644 --- a/eipw-lint/src/lints/markdown/heading_first.rs +++ b/eipw-lint/src/lints/markdown/heading_first.rs @@ -4,32 +4,46 @@ * file, You can obtain one at https://mozilla.org/MPL/2.0/. */ -use comrak::nodes::NodeValue; +use comrak::nodes::{Ast, NodeValue}; use crate::lints::{Error, Lint}; +use crate::SnippetExt; -#[derive(Debug)] +use eipw_snippets::Snippet; + +use serde::{Deserialize, Serialize}; + +#[derive(Debug, Clone, Serialize, Deserialize)] pub struct HeadingFirst; impl Lint for HeadingFirst { fn lint<'a>(&self, slug: &'a str, ctx: &crate::lints::Context<'a, '_>) -> Result<(), Error> { let second = match ctx.body().descendants().nth(1) { - Some(el) => el.data.borrow().to_owned().value, - None => { - return ctx.report( - ctx.annotation_level() - .title("Cannot submit an empty proposal") - .id(slug), - ) - } + Some(el) => el.data.borrow().to_owned(), + None => return Ok(()), + }; + + let ast = match second { + Ast { + value: NodeValue::Heading(_), + .. + } => return Ok(()), + other => other, }; - match second { - NodeValue::Heading(_) => Ok(()), - _ => ctx.report( - ctx.annotation_level() - .title("Nothing is permitted between the preamble and the first heading") - .id(slug), - ), - } + + let source = ctx.line(ast.sourcepos.start.line); + ctx.report( + ctx.annotation_level() + .title("Nothing is permitted between the preamble and the first heading") + .id(slug) + .snippet( + Snippet::source(source) + .origin_opt(ctx.origin()) + .line_start(ast.sourcepos.start.line) + .fold(false), + ), + )?; + + Ok(()) } } diff --git a/eipw-lint/tests/lint_markdown_heading_first.rs b/eipw-lint/tests/lint_markdown_heading_first.rs index 5a030829..b6d422f3 100644 --- a/eipw-lint/tests/lint_markdown_heading_first.rs +++ b/eipw-lint/tests/lint_markdown_heading_first.rs @@ -22,7 +22,7 @@ After the "Abstract" heading is the first place we want to allow text."#; let reports = Linter::>::default() .clear_lints() - .deny("markdown-headings-only", HeadingFirst {}) + .deny("markdown-heading-first", HeadingFirst {}) .check_slice(None, src) .run() .await @@ -30,8 +30,12 @@ After the "Abstract" heading is the first place we want to allow text."#; .into_inner(); assert_eq!( - reports.trim(), - "error[markdown-headings-only]: Nothing is permitted between the preamble and the first heading" + reports, + r#"error[markdown-heading-first]: Nothing is permitted between the preamble and the first heading + | +5 | This is some text that appears before the first heading. Authors sometimes try + | +"# ); } @@ -50,36 +54,11 @@ created: 2016-04-28 ### Specification Currently, the formula to compute the difficulty of a block includes the following logic: - -``` python -adj_factor = max(1 - ((timestamp - parent.timestamp) // 10), -99) -child_diff = int(max(parent.difficulty + (parent.difficulty // BLOCK_DIFF_FACTOR) * adj_factor, min(parent.difficulty, MIN_DIFF))) -... -``` - -If `block.number >= BYZANTIUM_FORK_BLKNUM`, we change the first line to the following: - -``` python -adj_factor = max((2 if len(parent.uncles) else 1) - ((timestamp - parent.timestamp) // 9), -99) -``` -### Rationale - -This new formula ensures that the difficulty adjustment algorithm targets a constant average rate of blocks produced including uncles, and so ensures a highly predictable issuance rate that cannot be manipulated upward by manipulating the uncle rate. A formula that accounts for the exact number of included uncles: -``` python -adj_factor = max(1 + len(parent.uncles) - ((timestamp - parent.timestamp) // 9), -99) -``` -can be fairly easily seen to be (to within a tolerance of ~3/4194304) mathematically equivalent to assuming that a block with `k` uncles is equivalent to a sequence of `k+1` blocks that all appear with the exact same timestamp, and this is likely the simplest possible way to accomplish the desired effect. But since the exact formula depends on the full block and not just the header, we are instead using an approximate formula that accomplishes almost the same effect but has the benefit that it depends only on the block header (as you can check the uncle hash against the blank hash). - -Changing the denominator from 10 to 9 ensures that the block time remains roughly the same (in fact, it should decrease by ~3% given the current uncle rate of 7%). - -### References - -1. EIP 100 issue and discussion: https://github.com/ethereum/EIPs/issues/100 -2. https://bitslog.wordpress.com/2016/04/28/uncle-mining-an-ethereum-consensus-protocol-flaw/"#; +"#; let reports = Linter::>::default() .clear_lints() - .deny("markdown-headings-only", HeadingFirst {}) + .deny("markdown-heading-first", HeadingFirst {}) .check_slice(None, src) .run() .await