Add additional diagnostics (#899)

* Refactor reporting of syntax errors
* Report diagnostics for unused or undefined labels
* Report undefined citation and unused entries
* Use indexed citations when processing requests
* Use new "unnecessary" diagnostic tag
* Add tests for diagnostics module
This commit is contained in:
Patrick Förster 2023-06-29 19:22:05 +02:00 committed by GitHub
parent 4c0fb5b039
commit d32de1548a
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
32 changed files with 1205 additions and 490 deletions

View file

@ -1,25 +0,0 @@
pub mod bib;
pub mod log;
pub mod tex;
use rowan::TextRange;
use syntax::BuildError;
#[derive(Debug, Clone)]
pub struct Diagnostic {
pub range: TextRange,
pub code: ErrorCode,
}
#[derive(Debug, Clone)]
pub enum ErrorCode {
UnexpectedRCurly,
RCurlyInserted,
MismatchedEnvironment,
ExpectingLCurly,
ExpectingKey,
ExpectingRCurly,
ExpectingEq,
ExpectingFieldValue,
Build(BuildError),
}

View file

@ -1,65 +0,0 @@
use rowan::{ast::AstNode, TextRange};
use syntax::bibtex::{self, HasDelims, HasEq, HasName, HasType, HasValue};
use crate::{Document, DocumentData};
use super::{Diagnostic, ErrorCode};
pub fn analyze(document: &mut Document) {
let DocumentData::Bib(data) = &document.data else { return };
for node in bibtex::SyntaxNode::new_root(data.green.clone()).descendants() {
if let Some(entry) = bibtex::Entry::cast(node.clone()) {
analyze_entry(document, entry);
} else if let Some(field) = bibtex::Field::cast(node.clone()) {
analyze_field(document, field);
}
}
}
fn analyze_entry(document: &mut Document, entry: bibtex::Entry) {
if entry.left_delim_token().is_none() {
document.diagnostics.push(Diagnostic {
range: entry.type_token().unwrap().text_range(),
code: ErrorCode::ExpectingLCurly,
});
return;
}
if entry.name_token().is_none() {
document.diagnostics.push(Diagnostic {
range: entry.left_delim_token().unwrap().text_range(),
code: ErrorCode::ExpectingKey,
});
return;
}
if entry.right_delim_token().is_none() {
document.diagnostics.push(Diagnostic {
range: TextRange::empty(entry.syntax().text_range().end()),
code: ErrorCode::ExpectingRCurly,
});
}
}
fn analyze_field(document: &mut Document, field: bibtex::Field) {
if field.eq_token().is_none() {
let code = ErrorCode::ExpectingEq;
document.diagnostics.push(Diagnostic {
range: field.name_token().unwrap().text_range(),
code,
});
return;
}
if field.value().is_none() {
let code = ErrorCode::ExpectingFieldValue;
document.diagnostics.push(Diagnostic {
range: field.name_token().unwrap().text_range(),
code,
});
}
}

View file

@ -1,68 +0,0 @@
use rowan::{TextLen, TextRange, TextSize};
use rustc_hash::FxHashMap;
use syntax::BuildError;
use url::Url;
use crate::{Document, DocumentData, Workspace};
use super::{Diagnostic, ErrorCode};
pub fn analyze<'a>(
workspace: &'a Workspace,
log_document: &'a Document,
) -> FxHashMap<&'a Document, Vec<Diagnostic>> {
let mut results = FxHashMap::default();
let DocumentData::Log(data) = &log_document.data else { return results };
let parents = workspace.parents(log_document);
let Some(root_document) = parents.iter().next() else { return results };
let Some(base_path) = root_document.path
.as_deref()
.and_then(|path| path.parent()) else { return results };
for error in &data.errors {
let full_path = base_path.join(&error.relative_path);
let Ok(full_path_uri) = Url::from_file_path(&full_path) else { continue };
let tex_document = workspace.lookup(&full_path_uri).unwrap_or(root_document);
let range = find_range_of_hint(tex_document, error).unwrap_or_else(|| {
let line = error.line.unwrap_or(0);
let offset = *tex_document
.line_index
.newlines
.get(line as usize)
.unwrap_or(&TextSize::from(0));
TextRange::empty(offset)
});
let diagnostic = Diagnostic {
range,
code: ErrorCode::Build(error.clone()),
};
results.entry(tex_document).or_default().push(diagnostic);
}
results
}
fn find_range_of_hint(document: &Document, error: &BuildError) -> Option<TextRange> {
let line = error.line? as usize;
let hint = error.hint.as_deref()?;
let line_index = &document.line_index;
let line_start = line_index.newlines.get(line).copied()?;
let line_end = line_index
.newlines
.get(line + 1)
.copied()
.unwrap_or((&document.text).text_len());
let line_text = &document.text[line_start.into()..line_end.into()];
let hint_start = line_start + TextSize::try_from(line_text.find(hint)?).unwrap();
let hint_end = hint_start + hint.text_len();
Some(TextRange::new(hint_start, hint_end))
}

View file

@ -1,116 +0,0 @@
use rowan::{ast::AstNode, NodeOrToken, TextRange};
use syntax::latex;
use crate::{Config, Document, DocumentData};
use super::{Diagnostic, ErrorCode};
pub fn analyze(document: &mut Document, config: &Config) {
if !document.uri.as_str().ends_with(".tex") {
return;
}
let DocumentData::Tex(data) = &document.data else { return };
let mut traversal = latex::SyntaxNode::new_root(data.green.clone()).preorder();
while let Some(event) = traversal.next() {
match event {
rowan::WalkEvent::Enter(node) => {
if let Some(environment) = latex::Environment::cast(node.clone()) {
if environment
.begin()
.and_then(|begin| begin.name())
.and_then(|name| name.key())
.map_or(false, |name| {
config
.syntax
.verbatim_environments
.contains(&name.to_string())
})
{
traversal.skip_subtree();
continue;
}
}
analyze_environment(document, node.clone())
.or_else(|| analyze_curly_group(document, node.clone(), config))
.or_else(|| analyze_curly_braces(document, node));
}
rowan::WalkEvent::Leave(_) => {
continue;
}
};
}
}
fn analyze_environment(document: &mut Document, node: latex::SyntaxNode) -> Option<()> {
let environment = latex::Environment::cast(node)?;
let begin = environment.begin()?.name()?.key()?;
let end = environment.end()?.name()?.key()?;
if begin != end {
document.diagnostics.push(Diagnostic {
range: latex::small_range(&begin),
code: ErrorCode::MismatchedEnvironment,
});
}
Some(())
}
fn analyze_curly_group(
document: &mut Document,
node: latex::SyntaxNode,
config: &Config,
) -> Option<()> {
if !matches!(
node.kind(),
latex::CURLY_GROUP
| latex::CURLY_GROUP_COMMAND
| latex::CURLY_GROUP_KEY_VALUE
| latex::CURLY_GROUP_WORD
| latex::CURLY_GROUP_WORD_LIST
) {
return None;
}
let is_inside_verbatim_environment = node
.ancestors()
.filter_map(latex::Environment::cast)
.filter_map(|env| env.begin())
.filter_map(|begin| begin.name())
.filter_map(|name| name.key())
.any(|name| {
config
.syntax
.verbatim_environments
.contains(&name.to_string())
});
if !is_inside_verbatim_environment
&& !node
.children_with_tokens()
.filter_map(NodeOrToken::into_token)
.any(|token| token.kind() == latex::R_CURLY)
{
document.diagnostics.push(Diagnostic {
range: TextRange::empty(node.text_range().end()),
code: ErrorCode::RCurlyInserted,
});
}
Some(())
}
fn analyze_curly_braces(document: &mut Document, node: latex::SyntaxNode) -> Option<()> {
if node.kind() == latex::ERROR && node.first_token()?.text() == "}" {
document.diagnostics.push(Diagnostic {
range: node.text_range(),
code: ErrorCode::UnexpectedRCurly,
});
Some(())
} else {
None
}
}

View file

@ -5,7 +5,6 @@ use syntax::{bibtex, latex, BuildError};
use url::Url;
use crate::{
diagnostics::{self, Diagnostic},
semantics,
util::{LineCol, LineIndex},
Config,
@ -17,6 +16,16 @@ pub enum Owner {
Server,
}
#[derive(Debug)]
pub struct DocumentParams<'a> {
pub uri: Url,
pub text: String,
pub language: Language,
pub owner: Owner,
pub cursor: LineCol,
pub config: &'a Config,
}
#[derive(Clone)]
pub struct Document {
pub uri: Url,
@ -28,18 +37,12 @@ pub struct Document {
pub cursor: LineCol,
pub language: Language,
pub data: DocumentData,
pub diagnostics: Vec<Diagnostic>,
}
impl Document {
pub fn parse(
uri: Url,
text: String,
language: Language,
owner: Owner,
cursor: LineCol,
config: &Config,
) -> Self {
pub fn parse(params: DocumentParams) -> Self {
let DocumentParams { uri, text, .. } = params;
let dir = uri.join(".").unwrap();
let path = if uri.scheme() == "file" {
@ -50,20 +53,21 @@ impl Document {
let line_index = LineIndex::new(&text);
let diagnostics = Vec::new();
let data = match language {
let data = match params.language {
Language::Tex => {
let green = parser::parse_latex(&text, &config.syntax);
let green = parser::parse_latex(&text, &params.config.syntax);
let mut semantics = semantics::tex::Semantics::default();
semantics.process_root(&latex::SyntaxNode::new_root(green.clone()));
DocumentData::Tex(TexDocumentData { green, semantics })
}
Language::Bib => {
let green = parser::parse_bibtex(&text);
DocumentData::Bib(BibDocumentData { green })
let mut semantics = semantics::bib::Semantics::default();
semantics.process_root(&bibtex::SyntaxNode::new_root(green.clone()));
DocumentData::Bib(BibDocumentData { green, semantics })
}
Language::Aux => {
let green = parser::parse_latex(&text, &config.syntax);
let green = parser::parse_latex(&text, &params.config.syntax);
let mut semantics = semantics::auxiliary::Semantics::default();
semantics.process_root(&latex::SyntaxNode::new_root(green.clone()));
DocumentData::Aux(AuxDocumentData { green, semantics })
@ -76,23 +80,16 @@ impl Document {
Language::Tectonic => DocumentData::Tectonic,
};
let mut document = Self {
let document = Self {
uri,
dir,
path,
text,
line_index,
owner,
cursor,
language,
owner: params.owner,
cursor: params.cursor,
language: params.language,
data,
diagnostics,
};
match language {
Language::Tex => diagnostics::tex::analyze(&mut document, config),
Language::Bib => diagnostics::bib::analyze(&mut document),
Language::Aux | Language::Log | Language::Root | Language::Tectonic => (),
};
document
@ -165,6 +162,14 @@ impl DocumentData {
None
}
}
pub fn as_log(&self) -> Option<&LogDocumentData> {
if let DocumentData::Log(data) = self {
Some(data)
} else {
None
}
}
}
#[derive(Debug, Clone)]
@ -182,6 +187,7 @@ impl TexDocumentData {
#[derive(Debug, Clone)]
pub struct BibDocumentData {
pub green: rowan::GreenNode,
pub semantics: semantics::bib::Semantics,
}
impl BibDocumentData {

View file

@ -1,6 +1,5 @@
mod config;
pub mod data;
pub mod diagnostics;
mod document;
pub mod graph;
pub mod semantics;

View file

@ -1,4 +1,5 @@
pub mod auxiliary;
pub mod bib;
pub mod tex;
#[derive(PartialEq, Eq, Clone, Hash)]
@ -6,6 +7,7 @@ pub struct Span {
pub text: String,
pub range: rowan::TextRange,
}
impl std::fmt::Debug for Span {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
f.debug_tuple("Span")
@ -17,9 +19,18 @@ impl std::fmt::Debug for Span {
impl From<&syntax::latex::Key> for Span {
fn from(key: &syntax::latex::Key) -> Self {
Span {
Self {
text: key.to_string(),
range: syntax::latex::small_range(key),
}
}
}
impl From<&syntax::bibtex::SyntaxToken> for Span {
fn from(token: &syntax::bibtex::SyntaxToken) -> Self {
Self {
text: token.text().into(),
range: token.text_range(),
}
}
}

View file

@ -0,0 +1,38 @@
use rowan::ast::AstNode;
use syntax::bibtex::{self, HasName};
use text_size::TextRange;
use super::Span;
#[derive(Debug, Clone, Default)]
pub struct Semantics {
pub entries: Vec<Entry>,
}
impl Semantics {
pub fn process_root(&mut self, root: &bibtex::SyntaxNode) {
for node in root.children() {
if let Some(entry) = bibtex::Entry::cast(node) {
self.process_entry(entry);
}
}
}
fn process_entry(&mut self, entry: bibtex::Entry) {
if let Some(name) = entry.name_token() {
self.entries.push(Entry {
name: Span {
range: name.text_range(),
text: name.text().into(),
},
full_range: entry.syntax().text_range(),
});
}
}
}
#[derive(Debug, Clone)]
pub struct Entry {
pub name: Span,
pub full_range: TextRange,
}

View file

@ -9,6 +9,7 @@ use super::Span;
pub struct Semantics {
pub links: Vec<Link>,
pub labels: Vec<Label>,
pub citations: Vec<Citation>,
pub commands: Vec<Span>,
pub environments: Vec<Span>,
pub theorem_definitions: Vec<TheoremDefinition>,
@ -53,6 +54,8 @@ impl Semantics {
self.process_label_reference(label);
} else if let Some(label) = latex::LabelReferenceRange::cast(node.clone()) {
self.process_label_reference_range(label);
} else if let Some(citation) = latex::Citation::cast(node.clone()) {
self.process_citation(citation);
} else if let Some(environment) = latex::Environment::cast(node.clone()) {
self.process_environment(environment);
} else if let Some(theorem_def) = latex::TheoremDefinition::cast(node.clone()) {
@ -209,6 +212,18 @@ impl Semantics {
}
}
fn process_citation(&mut self, citation: latex::Citation) {
let full_range = latex::small_range(&citation);
if let Some(list) = citation.key_list() {
for key in list.keys() {
self.citations.push(Citation {
name: Span::from(&key),
full_range,
});
}
}
}
fn process_environment(&mut self, environment: latex::Environment) {
let Some(name) = environment
.begin()
@ -298,3 +313,9 @@ pub struct TheoremDefinition {
pub name: Span,
pub heading: String,
}
#[derive(Debug, Clone)]
pub struct Citation {
pub name: Span,
pub full_range: TextRange,
}

View file

@ -10,7 +10,7 @@ use rustc_hash::FxHashMap;
#[derive(Clone, Debug, PartialEq, Eq)]
pub struct LineIndex {
/// Offset the the beginning of each line, zero-based
pub(crate) newlines: Vec<TextSize>,
pub newlines: Vec<TextSize>,
/// List of non-ASCII characters on each line
pub(crate) utf16_lines: FxHashMap<u32, Vec<Utf16Char>>,
}

View file

@ -10,7 +10,7 @@ use rustc_hash::FxHashSet;
use text_size::TextLen;
use url::Url;
use crate::{graph, util::LineCol, Config, Document, DocumentData, Owner};
use crate::{graph, util::LineCol, Config, Document, DocumentData, DocumentParams, Owner};
#[derive(Debug, Default)]
pub struct Workspace {
@ -56,14 +56,14 @@ impl Workspace {
) {
log::debug!("Opening document {uri}...");
self.documents.remove(&uri);
self.documents.insert(Document::parse(
self.documents.insert(Document::parse(DocumentParams {
uri,
text,
language,
owner,
cursor,
&self.config,
));
config: &self.config,
}));
}
pub fn load(&mut self, path: &Path, language: Language, owner: Owner) -> std::io::Result<()> {
@ -232,18 +232,18 @@ impl Workspace {
Some(())
}
pub fn discover(&mut self) {
pub fn discover(&mut self, checked_paths: &mut FxHashSet<PathBuf>) {
loop {
let mut changed = false;
changed |= self.discover_parents();
changed |= self.discover_children();
changed |= self.discover_parents(checked_paths);
changed |= self.discover_children(checked_paths);
if !changed {
break;
}
}
}
fn discover_parents(&mut self) -> bool {
fn discover_parents(&mut self, checked_paths: &mut FxHashSet<PathBuf>) -> bool {
let dirs = self
.iter()
.filter_map(|document| document.path.as_deref())
@ -278,6 +278,7 @@ impl Workspace {
if self.lookup_path(&file).is_none() {
changed |= self.load(&file, lang, Owner::Server).is_ok();
checked_paths.insert(file);
}
}
}
@ -285,8 +286,8 @@ impl Workspace {
changed
}
fn discover_children(&mut self) -> bool {
let paths = self
fn discover_children(&mut self, checked_paths: &mut FxHashSet<PathBuf>) -> bool {
let files = self
.iter()
.map(|start| graph::Graph::new(self, start))
.flat_map(|graph| graph.missing)
@ -295,10 +296,11 @@ impl Workspace {
.collect::<FxHashSet<_>>();
let mut changed = false;
for path in paths {
let language = Language::from_path(&path).unwrap_or(Language::Tex);
if self.lookup_path(&path).is_none() {
changed |= self.load(&path, language, Owner::Server).is_ok();
for file in files {
let language = Language::from_path(&file).unwrap_or(Language::Tex);
if self.lookup_path(&file).is_none() {
changed |= self.load(&file, language, Owner::Server).is_ok();
checked_paths.insert(file);
}
}