Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor: Merge LazyIssueSource into IssueSource #6433

Merged
merged 12 commits into from
Nov 13, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions crates/turbopack-core/src/issue/analyze.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ use anyhow::Result;
use turbo_tasks::Vc;
use turbo_tasks_fs::FileSystemPath;

use super::{Issue, IssueSeverity, LazyIssueSource, OptionIssueSource};
use super::{Issue, IssueSeverity, IssueSource, OptionIssueSource};
use crate::ident::AssetIdent;

#[turbo_tasks::value(shared)]
Expand All @@ -13,7 +13,7 @@ pub struct AnalyzeIssue {
pub message: Vc<String>,
pub category: Vc<String>,
pub code: Option<String>,
pub source: Option<Vc<LazyIssueSource>>,
pub source: Option<Vc<IssueSource>>,
}

#[turbo_tasks::value_impl]
Expand Down Expand Up @@ -49,6 +49,6 @@ impl Issue for AnalyzeIssue {

#[turbo_tasks::function]
fn source(&self) -> Vc<OptionIssueSource> {
Vc::cell(self.source.map(|s| s.to_issue_source()))
Vc::cell(self.source)
}
}
139 changes: 65 additions & 74 deletions crates/turbopack-core/src/issue/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -396,65 +396,18 @@ impl CapturedIssues {
}
}

/// Use this to pass and store byte offsets for an AST node along with its
/// source. When row/column is needed, this can be lazily converted into a
/// proper `IssueSource` at that time using
/// [LazyIssueSource::to_issue_source].
#[turbo_tasks::value]
#[derive(Clone, Debug)]
pub struct LazyIssueSource {
pub source: Vc<Box<dyn Source>>,
pub start_end: Option<(usize, usize)>,
}

#[turbo_tasks::value_impl]
impl LazyIssueSource {
/// Create a [`LazyIssueSource`] from byte offsets given by an swc ast node
/// span.
///
/// Arguments:
///
/// * `source`: The source code in which to look up the byte offsets.
/// * `start`: The start index of the span. Must use **1-based** indexing.
/// * `end`: The end index of the span. Must use **1-based** indexing.
#[turbo_tasks::function]
pub fn from_swc_offsets(source: Vc<Box<dyn Source>>, start: usize, end: usize) -> Vc<Self> {
match (start == 0, end == 0) {
(true, true) => Self::cell(LazyIssueSource {
source,
start_end: None,
}),
(false, false) => Self::cell(LazyIssueSource {
source,
start_end: Some((start - 1, end - 1)),
}),
(false, true) => Self::cell(LazyIssueSource {
source,
start_end: Some((start - 1, start - 1)),
}),
(true, false) => Self::cell(LazyIssueSource {
source,
start_end: Some((end - 1, end - 1)),
}),
}
}

#[turbo_tasks::function]
pub async fn to_issue_source(self: Vc<Self>) -> Result<Vc<IssueSource>> {
let this = &*self.await?;
Ok(if let Some((start, end)) = this.start_end {
IssueSource::from_byte_offset(this.source, start, end)
} else {
IssueSource::from_source_only(this.source)
})
}
pub struct IssueSource {
source: Vc<Box<dyn Source>>,
range: Option<Vc<SourceRange>>,
}

#[turbo_tasks::value]
#[derive(Clone, Debug)]
pub struct IssueSource {
source: Vc<Box<dyn Source>>,
range: Option<(SourcePos, SourcePos)>,
enum SourceRange {
LineColumn(SourcePos, SourcePos),
ByteOffset(usize, usize),
}

#[turbo_tasks::value_impl]
Expand All @@ -469,6 +422,27 @@ impl IssueSource {
})
}

/// Create a [`IssueSource`] from byte offsets given by an swc ast node
/// span.
///
/// Arguments:
///
/// * `source`: The source code in which to look up the byte offsets.
/// * `start`: The start index of the span. Must use **1-based** indexing.
/// * `end`: The end index of the span. Must use **1-based** indexing.
#[turbo_tasks::function]
pub fn from_swc_offsets(source: Vc<Box<dyn Source>>, start: usize, end: usize) -> Vc<Self> {
Self::cell(IssueSource {
source,
range: match (start == 0, end == 0) {
(true, true) => None,
(false, false) => Some(SourceRange::ByteOffset(start - 1, end - 1).cell()),
(false, true) => Some(SourceRange::ByteOffset(start - 1, start - 1).cell()),
(true, false) => Some(SourceRange::ByteOffset(end - 1, end - 1).cell()),
},
})
}

#[turbo_tasks::function]
/// Returns an `IssueSource` representing a span of code in the `source`.
/// Positions are derived from byte offsets and stored as lines and columns.
Expand All @@ -486,31 +460,12 @@ impl IssueSource {
start: usize,
end: usize,
) -> Result<Vc<Self>> {
fn find_line_and_column(lines: &[FileLine], offset: usize) -> SourcePos {
match lines.binary_search_by(|line| line.bytes_offset.cmp(&offset)) {
Ok(i) => SourcePos { line: i, column: 0 },
Err(i) => {
if i == 0 {
SourcePos {
line: 0,
column: offset,
}
} else {
let line = &lines[i - 1];
SourcePos {
line: i - 1,
column: min(line.content.len(), offset - line.bytes_offset),
}
}
}
}
}
Ok(Self::cell(IssueSource {
source,
range: if let FileLinesContent::Lines(lines) = &*source.content().lines().await? {
let start = find_line_and_column(lines.as_ref(), start);
let end = find_line_and_column(lines.as_ref(), end);
Some((start, end))
Some(SourceRange::LineColumn(start, end).cell())
} else {
None
},
Expand Down Expand Up @@ -615,7 +570,23 @@ impl IssueSource {
let this = self.await?;
Ok(PlainIssueSource {
asset: PlainSource::from_source(this.source).await?,
range: this.range,
range: match this.range {
Some(range) => match &*range.await? {
SourceRange::LineColumn(start, end) => Some((*start, *end)),
SourceRange::ByteOffset(start, end) => {
if let FileLinesContent::Lines(lines) =
&*this.source.content().lines().await?
{
let start = find_line_and_column(lines.as_ref(), *start);
let end = find_line_and_column(lines.as_ref(), *end);
Some((start, end))
} else {
None
}
}
},
_ => None,
},
}
.cell())
}
Expand Down Expand Up @@ -830,3 +801,23 @@ pub async fn handle_issues<T: Send>(
Ok(())
}
}

fn find_line_and_column(lines: &[FileLine], offset: usize) -> SourcePos {
match lines.binary_search_by(|line| line.bytes_offset.cmp(&offset)) {
Ok(i) => SourcePos { line: i, column: 0 },
Err(i) => {
if i == 0 {
SourcePos {
line: 0,
column: offset,
}
} else {
let line = &lines[i - 1];
SourcePos {
line: i - 1,
column: min(line.content.len(), offset - line.bytes_offset),
}
}
}
}
}
8 changes: 4 additions & 4 deletions crates/turbopack-core/src/issue/resolve.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,10 @@ use anyhow::Result;
use turbo_tasks::{ValueToString, Vc};
use turbo_tasks_fs::FileSystemPath;

use super::{Issue, OptionIssueSource};
use super::{Issue, IssueSource, OptionIssueSource};
use crate::{
error::PrettyPrintError,
issue::{IssueSeverity, LazyIssueSource},
issue::IssueSeverity,
resolve::{options::ResolveOptions, parse::Request},
};

Expand All @@ -19,7 +19,7 @@ pub struct ResolvingIssue {
pub file_path: Vc<FileSystemPath>,
pub resolve_options: Vc<ResolveOptions>,
pub error_message: Option<String>,
pub source: Option<Vc<LazyIssueSource>>,
pub source: Option<Vc<IssueSource>>,
}

#[turbo_tasks::value_impl]
Expand Down Expand Up @@ -102,7 +102,7 @@ impl Issue for ResolvingIssue {

#[turbo_tasks::function]
fn source(&self) -> Vc<OptionIssueSource> {
Vc::cell(self.source.map(|s| s.to_issue_source()))
Vc::cell(self.source)
}

// TODO add sub_issue for a description of resolve_options
Expand Down
4 changes: 2 additions & 2 deletions crates/turbopack-core/src/resolve/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ use self::{
};
use crate::{
file_source::FileSource,
issue::{resolve::ResolvingIssue, IssueExt, LazyIssueSource},
issue::{resolve::ResolvingIssue, IssueExt, IssueSource},
module::{Module, Modules, OptionModule},
output::{OutputAsset, OutputAssets},
package_json::{read_package_json, PackageJsonIssue},
Expand Down Expand Up @@ -1931,7 +1931,7 @@ pub async fn handle_resolve_error(
request: Vc<Request>,
resolve_options: Vc<ResolveOptions>,
severity: Vc<IssueSeverity>,
source: Option<Vc<LazyIssueSource>>,
source: Option<Vc<IssueSource>>,
) -> Result<Vc<ModuleResolveResult>> {
Ok(match result.is_unresolveable().await {
Ok(unresolveable) => {
Expand Down
6 changes: 3 additions & 3 deletions crates/turbopack-css/src/references/import.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ use swc_core::{
use turbo_tasks::{Value, ValueToString, Vc};
use turbopack_core::{
chunk::{ChunkableModuleReference, ChunkingContext},
issue::LazyIssueSource,
issue::IssueSource,
reference::ModuleReference,
reference_type::CssReferenceSubType,
resolve::{origin::ResolveOrigin, parse::Request, ModuleResolveResult},
Expand Down Expand Up @@ -187,7 +187,7 @@ pub struct ImportAssetReference {
pub request: Vc<Request>,
pub path: Vc<AstPath>,
pub attributes: Vc<ImportAttributes>,
pub issue_source: Vc<LazyIssueSource>,
pub issue_source: Vc<IssueSource>,
}

#[turbo_tasks::value_impl]
Expand All @@ -198,7 +198,7 @@ impl ImportAssetReference {
request: Vc<Request>,
path: Vc<AstPath>,
attributes: Vc<ImportAttributes>,
issue_source: Vc<LazyIssueSource>,
issue_source: Vc<IssueSource>,
) -> Vc<Self> {
Self::cell(ImportAssetReference {
origin,
Expand Down
8 changes: 4 additions & 4 deletions crates/turbopack-css/src/references/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ use swc_core::{
};
use turbo_tasks::{Value, Vc};
use turbopack_core::{
issue::{IssueSeverity, LazyIssueSource},
issue::{IssueSeverity, IssueSource},
reference::{ModuleReference, ModuleReferences},
reference_type::{CssReferenceSubType, ReferenceType},
resolve::{
Expand Down Expand Up @@ -133,7 +133,7 @@ impl<'a> VisitAstPath for ModuleReferencesVisitor<'a> {
Request::parse(Value::new(src.to_string().into())),
Vc::cell(as_parent_path(ast_path)),
ImportAttributes::new_from_prelude(i).into(),
LazyIssueSource::from_swc_offsets(
IssueSource::from_swc_offsets(
Vc::upcast(self.source),
issue_span.lo.to_usize(),
issue_span.hi.to_usize(),
Expand All @@ -160,7 +160,7 @@ impl<'a> VisitAstPath for ModuleReferencesVisitor<'a> {
self.origin,
Request::parse(Value::new(src.to_string().into())),
Vc::cell(as_parent_path(ast_path)),
LazyIssueSource::from_swc_offsets(
IssueSource::from_swc_offsets(
Vc::upcast(self.source),
issue_span.lo.to_usize(),
issue_span.hi.to_usize(),
Expand All @@ -177,7 +177,7 @@ pub async fn css_resolve(
origin: Vc<Box<dyn ResolveOrigin>>,
request: Vc<Request>,
ty: Value<CssReferenceSubType>,
issue_source: Option<Vc<LazyIssueSource>>,
issue_source: Option<Vc<IssueSource>>,
) -> Result<Vc<ModuleResolveResult>> {
let ty = Value::new(ReferenceType::Css(ty.into_value()));
let options = origin.resolve_options(ty.clone());
Expand Down
6 changes: 3 additions & 3 deletions crates/turbopack-css/src/references/url.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ use turbopack_core::{
ChunkingTypeOption,
},
ident::AssetIdent,
issue::{IssueSeverity, LazyIssueSource},
issue::{IssueSeverity, IssueSource},
output::OutputAsset,
reference::ModuleReference,
reference_type::UrlReferenceSubType,
Expand All @@ -37,7 +37,7 @@ pub struct UrlAssetReference {
pub origin: Vc<Box<dyn ResolveOrigin>>,
pub request: Vc<Request>,
pub path: Vc<AstPath>,
pub issue_source: Vc<LazyIssueSource>,
pub issue_source: Vc<IssueSource>,
}

#[turbo_tasks::value_impl]
Expand All @@ -47,7 +47,7 @@ impl UrlAssetReference {
origin: Vc<Box<dyn ResolveOrigin>>,
request: Vc<Request>,
path: Vc<AstPath>,
issue_source: Vc<LazyIssueSource>,
issue_source: Vc<IssueSource>,
) -> Vc<Self> {
Self::cell(UrlAssetReference {
origin,
Expand Down
6 changes: 3 additions & 3 deletions crates/turbopack-ecmascript/src/analyzer/imports.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ use swc_core::{
},
};
use turbo_tasks::Vc;
use turbopack_core::{issue::LazyIssueSource, source::Source};
use turbopack_core::{issue::IssueSource, source::Source};

use super::{JsValue, ModuleValue};
use crate::utils::unparen;
Expand Down Expand Up @@ -118,7 +118,7 @@ pub(crate) struct ImportMapReference {
pub module_path: JsWord,
pub imported_symbol: ImportedSymbol,
pub annotations: ImportAnnotations,
pub issue_source: Option<Vc<LazyIssueSource>>,
pub issue_source: Option<Vc<IssueSource>>,
}

impl ImportMap {
Expand Down Expand Up @@ -196,7 +196,7 @@ impl<'a> Analyzer<'a> {
) -> usize {
let issue_source = self
.source
.map(|s| LazyIssueSource::from_swc_offsets(s, span.lo.to_usize(), span.hi.to_usize()));
.map(|s| IssueSource::from_swc_offsets(s, span.lo.to_usize(), span.hi.to_usize()));

let r = ImportMapReference {
module_path,
Expand Down
Loading
Loading