Skip to content

Feature flags #1715

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

Merged
merged 2 commits into from
Aug 22, 2019
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
4 changes: 2 additions & 2 deletions crates/ra_batch/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ use std::{collections::HashSet, error::Error, path::Path};
use rustc_hash::FxHashMap;

use ra_db::{CrateGraph, FileId, SourceRootId};
use ra_ide_api::{AnalysisChange, AnalysisHost};
use ra_ide_api::{AnalysisChange, AnalysisHost, FeatureFlags};
use ra_project_model::{PackageRoot, ProjectWorkspace};
use ra_vfs::{RootEntry, Vfs, VfsChange};
use ra_vfs_glob::RustPackageFilterBuilder;
Expand Down Expand Up @@ -63,7 +63,7 @@ pub fn load(
vfs: &mut Vfs,
) -> AnalysisHost {
let lru_cap = std::env::var("RA_LRU_CAP").ok().and_then(|it| it.parse::<usize>().ok());
let mut host = AnalysisHost::new(lru_cap);
let mut host = AnalysisHost::new(lru_cap, FeatureFlags::default());
let mut analysis_change = AnalysisChange::new();
analysis_change.set_crate_graph(crate_graph);

Expand Down
5 changes: 4 additions & 1 deletion crates/ra_ide_api/src/completion/presentation.rs
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,10 @@ impl Completions {
.set_documentation(func.docs(ctx.db))
.detail(detail);
// If not an import, add parenthesis automatically.
if ctx.use_item_syntax.is_none() && !ctx.is_call {
if ctx.use_item_syntax.is_none()
&& !ctx.is_call
&& ctx.db.feature_flags.get("completion.insertion.add-call-parenthesis")
{
tested_by!(inserts_parens_for_function_calls);
let snippet =
if data.params().is_empty() || data.has_self_param() && data.params().len() == 1 {
Expand Down
9 changes: 6 additions & 3 deletions crates/ra_ide_api/src/db.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ use ra_db::{

use crate::{
symbol_index::{self, SymbolsDatabase},
LineIndex,
FeatureFlags, LineIndex,
};

#[salsa::database(
Expand All @@ -22,6 +22,7 @@ use crate::{
#[derive(Debug)]
pub(crate) struct RootDatabase {
runtime: salsa::Runtime<RootDatabase>,
pub(crate) feature_flags: Arc<FeatureFlags>,
pub(crate) last_gc: time::Instant,
pub(crate) last_gc_check: time::Instant,
}
Expand All @@ -46,16 +47,17 @@ impl salsa::Database for RootDatabase {

impl Default for RootDatabase {
fn default() -> RootDatabase {
RootDatabase::new(None)
RootDatabase::new(None, FeatureFlags::default())
}
}

impl RootDatabase {
pub fn new(lru_capacity: Option<usize>) -> RootDatabase {
pub fn new(lru_capacity: Option<usize>, feature_flags: FeatureFlags) -> RootDatabase {
let mut db = RootDatabase {
runtime: salsa::Runtime::default(),
last_gc: time::Instant::now(),
last_gc_check: time::Instant::now(),
feature_flags: Arc::new(feature_flags),
};
db.set_crate_graph_with_durability(Default::default(), Durability::HIGH);
db.set_local_roots_with_durability(Default::default(), Durability::HIGH);
Expand All @@ -74,6 +76,7 @@ impl salsa::ParallelDatabase for RootDatabase {
runtime: self.runtime.snapshot(self),
last_gc: self.last_gc,
last_gc_check: self.last_gc_check,
feature_flags: Arc::clone(&self.feature_flags),
})
}
}
Expand Down
67 changes: 67 additions & 0 deletions crates/ra_ide_api/src/feature_flags.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,67 @@
use rustc_hash::FxHashMap;

/// Feature flags hold fine-grained toggles for all *user-visible* features of
/// rust-analyzer.
///
/// The exists such that users are able to disable any annoying feature (and,
/// with many users and many features, some features are bound to be annoying
/// for some users)
///
/// Note that we purposefully use run-time checked strings, and not something
/// checked at compile time, to keep things simple and flexible.
///
/// Also note that, at the moment, `FeatureFlags` also store features for
/// `ra_lsp_server`. This should be benign layering violation.
#[derive(Debug)]
pub struct FeatureFlags {
flags: FxHashMap<String, bool>,
}

impl FeatureFlags {
fn new(flags: &[(&str, bool)]) -> FeatureFlags {
let flags = flags
.iter()
.map(|&(name, value)| {
check_flag_name(name);
(name.to_string(), value)
})
.collect();
FeatureFlags { flags }
}

pub fn set(&mut self, flag: &str, value: bool) -> Result<(), ()> {
match self.flags.get_mut(flag) {
None => Err(()),
Some(slot) => {
*slot = value;
Ok(())
}
}
}

pub fn get(&self, flag: &str) -> bool {
match self.flags.get(flag) {
None => panic!("unknown flag: {:?}", flag),
Some(value) => *value,
}
}
}

impl Default for FeatureFlags {
fn default() -> FeatureFlags {
FeatureFlags::new(&[
("lsp.diagnostics", true),
("completion.insertion.add-call-parenthesis", true),
("notifications.workspace-loaded", true),
])
}
}

fn check_flag_name(flag: &str) {
for c in flag.bytes() {
match c {
b'a'..=b'z' | b'-' | b'.' => (),
_ => panic!("flag name does not match conventions: {:?}", flag),
}
}
}
16 changes: 13 additions & 3 deletions crates/ra_ide_api/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ mod db;
pub mod mock_analysis;
mod symbol_index;
mod change;
mod feature_flags;

mod status;
mod completion;
Expand Down Expand Up @@ -63,6 +64,7 @@ pub use crate::{
completion::{CompletionItem, CompletionItemKind, InsertTextFormat},
diagnostics::Severity,
display::{file_structure, FunctionSignature, NavigationTarget, StructureNode},
feature_flags::FeatureFlags,
folding_ranges::{Fold, FoldKind},
hover::HoverResult,
inlay_hints::{InlayHint, InlayKind},
Expand Down Expand Up @@ -247,20 +249,24 @@ pub struct AnalysisHost {

impl Default for AnalysisHost {
fn default() -> AnalysisHost {
AnalysisHost::new(None)
AnalysisHost::new(None, FeatureFlags::default())
}
}

impl AnalysisHost {
pub fn new(lru_capcity: Option<usize>) -> AnalysisHost {
AnalysisHost { db: db::RootDatabase::new(lru_capcity) }
pub fn new(lru_capcity: Option<usize>, feature_flags: FeatureFlags) -> AnalysisHost {
AnalysisHost { db: db::RootDatabase::new(lru_capcity, feature_flags) }
}
/// Returns a snapshot of the current state, which you can query for
/// semantic information.
pub fn analysis(&self) -> Analysis {
Analysis { db: self.db.snapshot() }
}

pub fn feature_flags(&self) -> &FeatureFlags {
&self.db.feature_flags
}

/// Applies changes to the current state of the world. If there are
/// outstanding snapshots, they will be canceled.
pub fn apply_change(&mut self, change: AnalysisChange) {
Expand Down Expand Up @@ -319,6 +325,10 @@ impl Analysis {
(host.analysis(), file_id)
}

pub fn feature_flags(&self) -> &FeatureFlags {
&self.db.feature_flags
}

/// Debug info about the current state of the analysis
pub fn status(&self) -> Cancelable<String> {
self.with_db(|db| status::status(&*db))
Expand Down
13 changes: 6 additions & 7 deletions crates/ra_lsp_server/src/config.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
use rustc_hash::FxHashMap;

use serde::{Deserialize, Deserializer};

/// Client provided initialization options
Expand All @@ -12,29 +14,26 @@ pub struct ServerConfig {
#[serde(deserialize_with = "nullable_bool_false")]
pub publish_decorations: bool,

/// Whether or not the workspace loaded notification should be sent
///
/// Defaults to `true`
#[serde(deserialize_with = "nullable_bool_true")]
pub show_workspace_loaded: bool,

pub exclude_globs: Vec<String>,

pub lru_capacity: Option<usize>,

/// For internal usage to make integrated tests faster.
#[serde(deserialize_with = "nullable_bool_true")]
pub with_sysroot: bool,

/// Fine grained feature flags to disable specific features.
pub feature_flags: FxHashMap<String, bool>,
}

impl Default for ServerConfig {
fn default() -> ServerConfig {
ServerConfig {
publish_decorations: false,
show_workspace_loaded: true,
exclude_globs: Vec::new(),
lru_capacity: None,
with_sysroot: true,
feature_flags: FxHashMap::default(),
}
}
}
Expand Down
5 changes: 4 additions & 1 deletion crates/ra_lsp_server/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,5 +11,8 @@ mod world;

pub type Result<T> = std::result::Result<T, Box<dyn std::error::Error + Send + Sync>>;
pub use crate::{
caps::server_capabilities, config::ServerConfig, main_loop::main_loop, main_loop::LspError,
caps::server_capabilities,
config::ServerConfig,
main_loop::LspError,
main_loop::{main_loop, show_message},
};
19 changes: 13 additions & 6 deletions crates/ra_lsp_server/src/main.rs
Original file line number Diff line number Diff line change
@@ -1,8 +1,7 @@
use flexi_logger::{Duplicate, Logger};
use gen_lsp_server::{run_server, stdio_transport};
use serde::Deserialize;

use ra_lsp_server::{Result, ServerConfig};
use ra_lsp_server::{show_message, Result, ServerConfig};
use ra_prof;

fn main() -> Result<()> {
Expand Down Expand Up @@ -46,15 +45,23 @@ fn main_inner() -> Result<()> {
.filter(|workspaces| !workspaces.is_empty())
.unwrap_or_else(|| vec![root]);

let opts = params
let server_config: ServerConfig = params
.initialization_options
.and_then(|v| {
ServerConfig::deserialize(v)
.map_err(|e| log::error!("failed to deserialize config: {}", e))
serde_json::from_value(v)
.map_err(|e| {
log::error!("failed to deserialize config: {}", e);
show_message(
lsp_types::MessageType::Error,
format!("failed to deserialize config: {}", e),
s,
);
})
.ok()
})
.unwrap_or_default();
ra_lsp_server::main_loop(workspace_roots, params.capabilities, opts, r, s)

ra_lsp_server::main_loop(workspace_roots, params.capabilities, server_config, r, s)
})?;
log::info!("shutting down IO...");
threads.join()?;
Expand Down
48 changes: 35 additions & 13 deletions crates/ra_lsp_server/src/main_loop.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ use gen_lsp_server::{
handle_shutdown, ErrorCode, RawMessage, RawNotification, RawRequest, RawResponse,
};
use lsp_types::{ClientCapabilities, NumberOrString};
use ra_ide_api::{Canceled, FileId, LibraryData};
use ra_ide_api::{Canceled, FeatureFlags, FileId, LibraryData};
use ra_prof::profile;
use ra_vfs::VfsTask;
use serde::{de::DeserializeOwned, Serialize};
Expand Down Expand Up @@ -56,7 +56,7 @@ pub fn main_loop(
msg_receiver: &Receiver<RawMessage>,
msg_sender: &Sender<RawMessage>,
) -> Result<()> {
log::debug!("server_config: {:?}", config);
log::info!("server_config: {:#?}", config);
// FIXME: support dynamic workspace loading.
let workspaces = {
let ws_worker = workspace_loader(config.with_sysroot);
Expand All @@ -83,20 +83,35 @@ pub fn main_loop(
.iter()
.map(|glob| ra_vfs_glob::Glob::new(glob))
.collect::<std::result::Result<Vec<_>, _>>()?;
let feature_flags = {
let mut ff = FeatureFlags::default();
for (flag, value) in config.feature_flags {
if let Err(_) = ff.set(flag.as_str(), value) {
log::error!("unknown feature flag: {:?}", flag);
show_message(
req::MessageType::Error,
format!("unknown feature flag: {:?}", flag),
msg_sender,
);
}
}
ff
};
log::info!("feature_flags: {:#?}", feature_flags);
let mut state = WorldState::new(
ws_roots,
workspaces,
config.lru_capacity,
&globs,
Options {
publish_decorations: config.publish_decorations,
show_workspace_loaded: config.show_workspace_loaded,
supports_location_link: client_caps
.text_document
.and_then(|it| it.definition)
.and_then(|it| it.link_support)
.unwrap_or(false),
},
feature_flags,
);

let pool = ThreadPool::new(THREADPOOL_SIZE);
Expand Down Expand Up @@ -276,7 +291,7 @@ fn main_loop_inner(
&& in_flight_libraries == 0
{
let n_packages: usize = state.workspaces.iter().map(|it| it.n_packages()).sum();
if state.options.show_workspace_loaded {
if state.feature_flags().get("notifications.workspace-loaded") {
let msg = format!("workspace loaded, {} rust packages", n_packages);
show_message(req::MessageType::Info, msg, msg_sender);
}
Expand Down Expand Up @@ -587,17 +602,20 @@ fn update_file_notifications_on_threadpool(
subscriptions: Vec<FileId>,
) {
log::trace!("updating notifications for {:?}", subscriptions);
let publish_diagnostics = world.feature_flags().get("lsp.diagnostics");
pool.execute(move || {
Copy link
Contributor

Choose a reason for hiding this comment

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

Couldn't you do a pre-spawn condition check whether this will actually do something? This is in the main thread and loop, and maybe we don't even do something, yet we spawn a useless thread (iterating over files for nothing)?

Copy link
Member Author

Choose a reason for hiding this comment

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

This doesn’t spawn a thread, just sends a closure over a channel, it”s only a constant amount of work, comparable to the work we need to get to this point in the first place.

for file_id in subscriptions {
match handlers::publish_diagnostics(&world, file_id) {
Err(e) => {
if !is_canceled(&e) {
log::error!("failed to compute diagnostics: {:?}", e);
if publish_diagnostics {
match handlers::publish_diagnostics(&world, file_id) {
Err(e) => {
if !is_canceled(&e) {
log::error!("failed to compute diagnostics: {:?}", e);
}
}
Ok(params) => {
let not = RawNotification::new::<req::PublishDiagnostics>(&params);
sender.send(Task::Notify(not)).unwrap();
}
}
Ok(params) => {
let not = RawNotification::new::<req::PublishDiagnostics>(&params);
sender.send(Task::Notify(not)).unwrap();
}
}
if publish_decorations {
Expand All @@ -617,7 +635,11 @@ fn update_file_notifications_on_threadpool(
});
}

fn show_message(typ: req::MessageType, message: impl Into<String>, sender: &Sender<RawMessage>) {
pub fn show_message(
typ: req::MessageType,
message: impl Into<String>,
sender: &Sender<RawMessage>,
) {
let message = message.into();
let params = req::ShowMessageParams { typ, message };
let not = RawNotification::new::<req::ShowMessage>(&params);
Expand Down
Loading