Skip to content
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
140 changes: 138 additions & 2 deletions crates/ide-diagnostics/src/handlers/data_exchange_loading.rs
Original file line number Diff line number Diff line change
@@ -1,8 +1,10 @@
use crate::define_metadata;
use crate::metadata::*;
use crate::{Diagnostic, DiagnosticCode, DiagnosticsContext};
use crate::{Diagnostic, DiagnosticCode, DiagnosticsContext, Fix, TextEdit};
use hir::{Body, Expr, ExprId, IdConversion, Name, Stmt, StmtId};
use ide_db::TextRange;
use stdx::case::CaseExt;
use syntax::{SyntaxKind, SyntaxNode, SyntaxToken, TextSize};

pub const METADATA: DiagnosticMetadata = define_metadata! {
diagnostic_type: DiagnosticType::Error,
Expand Down Expand Up @@ -57,6 +59,7 @@ pub fn check(ctx: &DiagnosticsContext) -> Vec<Diagnostic> {

let item_tree = ctx.item_tree();
let module_bodies = ctx.module_bodies();
let root = ctx.parse().syntax_node();

let mut diagnostics = Vec::new();

Expand All @@ -74,14 +77,147 @@ pub fn check(ctx: &DiagnosticsContext) -> Vec<Diagnostic> {
severity: ctx.severity(code),
range: proc.name_range(),
tags: ctx.tags(code),
fixes: vec![],
fixes: insert_guard_fix(&root, proc.source_range()),
});
}
}

diagnostics
}

/// Inserts `Если ОбменДанными.Загрузка Тогда Возврат; КонецЕсли;` as the first
/// statement of the handler, after its local `Перем` declarations (they must precede
/// every statement), indented like the body. Manual: during data exchange the
/// handler then stops running its logic, which is the point of the check but still
/// a change of behaviour the author has to accept.
fn insert_guard_fix(root: &SyntaxNode, source_range: TextRange) -> Vec<Fix> {
let Some(method) = root
.descendants()
.find(|node| node.text_range() == source_range && node.kind() == SyntaxKind::PROCEDURE_DEF)
else {
return Vec::new();
};
let english = method
.children_with_tokens()
.filter_map(|element| element.into_token())
.find(|token| token.kind() == SyntaxKind::KW_PROCEDURE)
.is_some_and(|keyword| keyword.text().is_ascii());

let items: Vec<SyntaxNode> = method
.children()
.find(|node| node.kind() == SyntaxKind::STMT_LIST)
.iter()
.flat_map(|list| list.children())
.collect();
let declarations_end = items.iter().filter_map(body_declarations_end).max();
let first_statement = items.iter().find(|item| executes(item));

let Line { start, indent, newline } = match first_statement {
// A body declaration after the first statement (they share a directive, or the
// body is out of order) leaves no line that follows every declaration and
// precedes every statement.
Some(stmt) if declarations_end.is_some_and(|end| stmt.text_range().start() < end) => {
return Vec::new();
}
Some(stmt) => {
// A statement sharing its line with the header or another statement has
// no line of its own to put the guard above.
let Some(line) = stmt.first_token().and_then(|token| own_line(&token)) else {
return Vec::new();
};
line
}
None => {
let Some(line) = method.last_token().and_then(|token| own_line(&token)) else {
return Vec::new();
};
Line { indent: "\t".to_string(), ..line }
}
};

let unit = if indent.is_empty() || indent.contains('\t') { "\t" } else { indent.as_str() };
let (if_kw, condition, then_kw, return_kw, end_if) = if english {
("If", "DataExchange.Load", "Then", "Return;", "EndIf;")
} else {
("Если", "ОбменДанными.Загрузка", "Тогда", "Возврат;", "КонецЕсли;")
};
let guard = format!(
"{indent}{if_kw} {condition} {then_kw}{newline}\
{indent}{unit}{return_kw}{newline}\
{indent}{end_if}{newline}"
);
vec![Fix::manual(
format!("Добавить проверку {condition}"),
vec![TextEdit { range: TextRange::empty(start), new_text: guard }],
)]
}

/// A `#Если` directive and its branches hold their code as their own children, so
/// a declaration or statement inside them stays at the level of the body.
fn is_directive_branch(node: &SyntaxNode) -> bool {
matches!(
node.kind(),
SyntaxKind::PRE_IF_DIR | SyntaxKind::PRE_ELSIF_CLAUSE | SyntaxKind::PRE_ELSE_CLAUSE
)
}

/// End of the last `Перем` that `item` declares at the level of the body. A `Перем`
/// nested in an ordinary block such as `Если` is not one: the guard may precede the
/// whole block.
fn body_declarations_end(item: &SyntaxNode) -> Option<TextSize> {
match item.kind() {
SyntaxKind::VAR_DEF => Some(item.text_range().end()),
_ if is_directive_branch(item) => {
item.children().filter_map(|child| body_declarations_end(&child)).max()
}
_ => None,
}
}

/// Whether `item` runs code at the level of the body. Region markers and the
/// condition of a directive run nothing.
fn executes(item: &SyntaxNode) -> bool {
match item.kind() {
SyntaxKind::VAR_DEF | SyntaxKind::PRE_REGION_DIR | SyntaxKind::PRE_EXPR => false,
_ if is_directive_branch(item) => item.children().any(|child| executes(&child)),
_ => true,
}
}

struct Line {
start: TextSize,
indent: String,
/// Line break of the line above, so the inserted lines follow the file's style.
newline: String,
}

/// The line `token` begins, when only whitespace precedes it on that line.
fn own_line(token: &SyntaxToken) -> Option<Line> {
let mut indent = String::new();
let mut start = token.text_range().start();
let mut prev = token.prev_token();
while let Some(candidate) = prev {
match candidate.kind() {
SyntaxKind::WHITESPACE if !candidate.text().contains(['\n', '\r']) => {
indent.insert_str(0, candidate.text());
start = candidate.text_range().start();
prev = candidate.prev_token();
}
SyntaxKind::NEWLINE => {
// The lexer leaves the `\r` of a CRLF break in the token before `\n`.
let crlf = candidate.text().contains('\r')
|| candidate.prev_token().is_some_and(|before| before.text().ends_with('\r'));
let newline = if crlf { "\r\n" } else { "\n" }.to_string();
return Some(Line { start, indent, newline });
}
// The byte order mark opens the file without being text of its first line.
SyntaxKind::BOM => break,
_ => return None,
}
}
Some(Line { start, indent, newline: "\n".to_string() })
}

fn is_applicable_module(ctx: &DiagnosticsContext) -> bool {
let file_path = match ctx.file_path() {
Some(path) => path,
Expand Down
171 changes: 168 additions & 3 deletions crates/ide-diagnostics/src/handlers/unused_local_method.rs
Original file line number Diff line number Diff line change
@@ -1,10 +1,11 @@
use crate::define_metadata;
use crate::metadata::*;
use crate::utils::platform_event_handlers as peh;
use crate::{Diagnostic, DiagnosticCode, DiagnosticsContext};
use crate::{Diagnostic, DiagnosticCode, DiagnosticsContext, Fix, TextEdit};
use ide_db::TextRange;
use rustc_hash::FxHashSet;
use stdx::case::CaseExt;
use syntax::{SyntaxKind, SyntaxNode, SyntaxToken, TextSize};

pub const METADATA: DiagnosticMetadata = define_metadata! {
diagnostic_type: DiagnosticType::CodeSmell,
Expand Down Expand Up @@ -154,8 +155,11 @@ pub fn check(ctx: &DiagnosticsContext) -> Vec<Diagnostic> {
mdo_type: metadata.mdo.as_ref().map(|mdo| mdo.mdo_type),
};

let root = ctx.parse().syntax_node();
let string_literals = string_literal_texts(&root);

for (_, proc) in item_tree.procedures() {
if let Some(diag) = check_method_unused(
if let Some(mut diag) = check_method_unused(
&proc.name,
proc.name_range,
proc.is_export,
Expand All @@ -166,12 +170,13 @@ pub fn check(ctx: &DiagnosticsContext) -> Vec<Diagnostic> {
code,
ctx,
) {
diag.fixes = remove_method_fix(&root, proc.source_range, &proc.name, &string_literals);
diagnostics.push(diag);
}
}

for (_, func) in item_tree.functions() {
if let Some(diag) = check_method_unused(
if let Some(mut diag) = check_method_unused(
&func.name,
func.name_range,
func.is_export,
Expand All @@ -182,13 +187,173 @@ pub fn check(ctx: &DiagnosticsContext) -> Vec<Diagnostic> {
code,
ctx,
) {
diag.fixes = remove_method_fix(&root, func.source_range, &func.name, &string_literals);
diagnostics.push(diag);
}
}

diagnostics
}

/// Deletes the method together with the doc comment directly above it and the rest
/// of its closing line. Manual: the method may still be reached by its name in a
/// string (`Выполнить`, a handler bound by name in another module), which the
/// analysis cannot see. When the name does appear in a string literal of the module,
/// no fix is offered at all.
fn remove_method_fix(
root: &SyntaxNode,
source_range: TextRange,
name: &hir::Name,
string_literals: &[String],
) -> Vec<Fix> {
let name_lower = name.as_str().fold_lower();
if string_literals.iter().any(|literal| mentions_identifier(literal, &name_lower)) {
return Vec::new();
}
let Some(method) = root.descendants().find(|node| {
node.text_range() == source_range
&& matches!(node.kind(), SyntaxKind::PROCEDURE_DEF | SyntaxKind::FUNCTION_DEF)
}) else {
return Vec::new();
};
let (Some(first), Some(last)) = (method.first_token(), method.last_token()) else {
return Vec::new();
};
let Some(mut start) = line_start(&first) else { return Vec::new() };
let mut blank_above = true;
let mut above = line_above(&first);
while let Some(line) = above {
match line {
LineAbove::Comment(comment) => {
start = line_start(&comment).unwrap_or(start);
above = line_above(&comment);
}
LineAbove::Blank => break,
LineAbove::Code => {
blank_above = false;
break;
}
}
}

// The closing keyword's own line: trailing blanks, a trailing comment, its break.
let mut end = last.text_range().end();
let mut next = last.next_token();
while let Some(token) = next.clone() {
match token.kind() {
SyntaxKind::WHITESPACE | SyntaxKind::COMMENT => {
end = token.text_range().end();
next = token.next_token();
}
SyntaxKind::NEWLINE => {
end = token.text_range().end();
next = token.next_token();
break;
}
_ => break,
}
}
// One blank line below goes too when one stays above, so the removal leaves a
// single blank line rather than two.
if blank_above {
let mut probe = next;
while let Some(token) = probe {
match token.kind() {
SyntaxKind::WHITESPACE => probe = token.next_token(),
SyntaxKind::NEWLINE => {
end = token.text_range().end();
break;
}
_ => break,
}
}
}

vec![Fix::manual(
format!("Удалить неиспользуемый метод \"{}\"", name.as_str()),
vec![TextEdit { range: TextRange::new(start, end), new_text: String::new() }],
)]
}

enum LineAbove {
Comment(SyntaxToken),
Blank,
Code,
}

/// What stands on the line above the one `token` begins. `None` at the top of the
/// file; a comment counts only when it owns its line.
fn line_above(token: &SyntaxToken) -> Option<LineAbove> {
let mut prev = token.prev_token();
while let Some(candidate) = prev.clone() {
match candidate.kind() {
SyntaxKind::WHITESPACE => prev = candidate.prev_token(),
SyntaxKind::NEWLINE => break,
// The byte order mark opens the file without being text of its first line.
SyntaxKind::BOM => return None,
_ => return Some(LineAbove::Code),
}
}
let newline = prev?;
let mut prev = newline.prev_token();
while let Some(candidate) = prev.clone() {
if candidate.kind() == SyntaxKind::WHITESPACE {
prev = candidate.prev_token();
} else {
break;
}
}
let Some(token) = prev else { return Some(LineAbove::Blank) };
match token.kind() {
SyntaxKind::NEWLINE | SyntaxKind::BOM => Some(LineAbove::Blank),
SyntaxKind::COMMENT if line_start(&token).is_some() => Some(LineAbove::Comment(token)),
_ => Some(LineAbove::Code),
}
}

/// Start of the line `token` begins, when only whitespace precedes it on that line.
fn line_start(token: &SyntaxToken) -> Option<TextSize> {
let mut start = token.text_range().start();
let mut prev = token.prev_token();
while let Some(candidate) = prev {
match candidate.kind() {
SyntaxKind::WHITESPACE => {
start = candidate.text_range().start();
prev = candidate.prev_token();
}
SyntaxKind::NEWLINE | SyntaxKind::BOM => return Some(start),
_ => return None,
}
}
Some(start)
}

fn string_literal_texts(root: &SyntaxNode) -> Vec<String> {
root.descendants_with_tokens()
.filter_map(|element| element.into_token())
.filter(|token| {
matches!(
token.kind(),
SyntaxKind::STRING
| SyntaxKind::STRING_START
| SyntaxKind::STRING_PART
| SyntaxKind::STRING_TAIL
)
})
.map(|token| token.text().fold_lower())
.collect()
}

/// `name_lower` occurs in `text` as a whole identifier, not as a part of a longer one.
fn mentions_identifier(text: &str, name_lower: &str) -> bool {
let is_ident = |c: char| c.is_alphanumeric() || c == '_';
text.match_indices(name_lower).any(|(at, found)| {
let before = text[..at].chars().next_back();
let after = text[at + found.len()..].chars().next();
!before.is_some_and(is_ident) && !after.is_some_and(is_ident)
})
}

#[allow(clippy::too_many_arguments)]
fn check_method_unused(
name: &hir::Name,
Expand Down
Loading
Loading