Skip to content

Commit

Permalink
refactor: Merge LazyIssueSource into IssueSource (vercel/turborep…
Browse files Browse the repository at this point in the history
…o#6433)

### Description

This is required for vercel/turborepo#5737


### Testing Instructions



Closes PACK-1953

---------

Co-authored-by: Tobias Koppers <[email protected]>
  • Loading branch information
kdy1 and sokra authored Nov 13, 2023
1 parent ef8e469 commit 9acd6d5
Show file tree
Hide file tree
Showing 17 changed files with 124 additions and 137 deletions.
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

0 comments on commit 9acd6d5

Please sign in to comment.