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

Provide package symbols before lint in diagnostics #568

Merged
merged 12 commits into from
Sep 25, 2022
2 changes: 2 additions & 0 deletions NAMESPACE
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
# Generated by roxygen2: do not edit by hand

export(run)
import(callr)
import(xml2)
importFrom(R6,R6Class)
importFrom(parallel,detectCores)
useDynLib(languageserver)
28 changes: 26 additions & 2 deletions R/diagnostics.R
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ find_config <- function(filename) {
#'
#' Lint and diagnose problems in a file.
#' @noRd
diagnose_file <- function(uri, is_rmarkdown, content, cache = FALSE) {
diagnose_file <- function(uri, content, is_rmarkdown = FALSE, globals = NULL, cache = FALSE) {
if (length(content) == 0) {
return(list())
}
Expand All @@ -84,6 +84,12 @@ diagnose_file <- function(uri, is_rmarkdown, content, cache = FALSE) {
content <- c(content, "")
}

if (length(globals)) {
env_name <- "languageserver:globals"
attach(globals, name = env_name)
on.exit(detach(env_name, character.only = TRUE))
}

lints <- lintr::lint(path, cache = cache, text = content)
diagnostics <- lapply(lints, diagnostic_from_lint, content = content)
names(diagnostics) <- NULL
Expand Down Expand Up @@ -113,12 +119,30 @@ diagnostics_callback <- function(self, uri, version, diagnostics) {
diagnostics_task <- function(self, uri, document, delay = 0) {
version <- document$version
content <- document$content

is_package <- is_package(self$rootPath)
globals <- NULL

if (is_package) {
globals <- new.env(parent = emptyenv())
renkun-ken marked this conversation as resolved.
Show resolved Hide resolved
for (doc in self$workspace$documents$values()) {
if (dirname(path_from_uri(doc$uri)) != file.path(self$rootPath, "R")) next
parse_data <- doc$parse_data
if (is.null(parse_data)) next
for (symbol in parse_data$nonfuncts) {
globals[[symbol]] <- NULL
}
list2env(parse_data$functions, globals)
}
}

create_task(
target = package_call(diagnose_file),
args = list(
uri = uri,
is_rmarkdown = document$is_rmarkdown,
content = content,
is_rmarkdown = document$is_rmarkdown,
globals = globals,
cache = lsp_settings$get("lint_cache")
),
callback = function(result) diagnostics_callback(self, uri, version, result),
Expand Down
6 changes: 4 additions & 2 deletions R/document.R
Original file line number Diff line number Diff line change
Expand Up @@ -362,7 +362,9 @@ parse_expr <- function(content, expr, env, srcref = attr(expr, "srcref")) {

if (type == "function") {
env$functs <- c(env$functs, symbol)
env$formals[[symbol]] <- value[[2L]]
fun <- function() NULL
formals(fun) <- value[[2L]]
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

parse_data now preserves the functions defined in the documents with only formals, which could make it easier to work with. If a future version of lintr checks if function arguments supplied match the function formals, it should be still working as the formals are preserved.

Also, it make it easier to works with match.call(fun, expr) in the future.

env$functions[[symbol]] <- fun
env$signatures[[symbol]] <- get_signature(symbol, value)
} else {
env$nonfuncts <- c(env$nonfuncts, symbol)
Expand Down Expand Up @@ -408,7 +410,7 @@ parse_document <- function(uri, content) {
env$packages <- character()
env$nonfuncts <- character()
env$functs <- character()
env$formals <- list()
env$functions <- list()
env$signatures <- list()
env$definitions <- list()
env$documentation <- list()
Expand Down
3 changes: 1 addition & 2 deletions R/handlers-general.R
Original file line number Diff line number Diff line change
Expand Up @@ -49,8 +49,7 @@ on_initialize <- function(self, id, params) {
#' @noRd
on_initialized <- function(self, params) {
logger$info("on_initialized")
project_root <- self$rootPath
if (length(project_root) && is_package(project_root)) {
if (is_package(self$rootPath)) {
# a bit like devtools::load_all()
self$workspace$load_all(self)
# TODO: result lint result of the package
Expand Down
2 changes: 1 addition & 1 deletion R/handlers-workspace.R
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ workspace_did_change_watched_files <- function(self, params) {
# Only non-open documents in a package should be handled here.

project_root <- self$rootPath
if (length(project_root) && is_package(project_root)) {
if (is_package(project_root)) {
source_dir <- file.path(project_root, "R")
for (file_event in params$changes) {
uri <- file_event$uri
Expand Down
1 change: 0 additions & 1 deletion R/languagebase.R
Original file line number Diff line number Diff line change
Expand Up @@ -158,7 +158,6 @@ LanguageBase <- R6::R6Class("LanguageBase",
},

handle_notification = function(notification) {
# logger$info("receive notification: ", notification)
method <- notification$method
params <- notification$params
if (method %in% names(self$notification_handlers)) {
Expand Down
1 change: 1 addition & 0 deletions R/languageserver.R
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
#' @useDynLib languageserver
#' @importFrom R6 R6Class
#' @import xml2
#' @importFrom parallel detectCores
#' @details
#' An implementation of the Language Server Protocol for R
"_PACKAGE"
Expand Down
2 changes: 1 addition & 1 deletion R/namespace.R
Original file line number Diff line number Diff line change
Expand Up @@ -244,7 +244,7 @@ GlobalEnv <- R6::R6Class("GlobalEnv",
for (doc in self$documents$values()) {
if (!is.null(doc$parse_data)) {
if (funct %in% doc$parse_data$functs) {
return(doc$parse_data$formals[[funct]])
return(formals(doc$parse_data$functions[[funct]]))
}
}
}
Expand Down
2 changes: 1 addition & 1 deletion R/session.R
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
#' Single R Session for Session Pool
#'
#' @import callr
#' @examples
#' \dontrun{
#' # create a session and bind to a session pool
Expand Down
2 changes: 1 addition & 1 deletion R/utils.R
Original file line number Diff line number Diff line change
Expand Up @@ -325,7 +325,7 @@ find_package <- function(path = getwd()) {
#' @noRd
is_package <- function(rootPath) {
file <- file.path(rootPath, "DESCRIPTION")
file.exists(file) && !dir.exists(file)
length(file) && file.exists(file) && !dir.exists(file)
}

get_root_path_for_uri <- function(uri, rootPath) {
Expand Down