4 files changed, 69 insertions, 31 deletions

diff --git a/crates/ra_syntax/src/ast/edit.rs b/crates/ra_syntax/src/ast/edit.rs
index 47bdbb81a..6f005a2d8 100644
--- a/crates/ra_syntax/src/ast/edit.rs
+++ b/crates/ra_syntax/src/ast/edit.rs
@@ -358,7 +358,7 @@ fn replace_children<N: AstNode>(
 fn test_increase_indent() {
     let arm_list = {
         let arm = make::match_arm(iter::once(make::placeholder_pat().into()), make::expr_unit());
-        make::match_arm_list(vec![arm.clone(), arm].into_iter())
+        make::match_arm_list(vec![arm.clone(), arm])
     };
     assert_eq!(
         arm_list.syntax().to_string(),
diff --git a/crates/ra_syntax/src/ast/make.rs b/crates/ra_syntax/src/ast/make.rs
index 95062ef6c..9749327fa 100644
--- a/crates/ra_syntax/src/ast/make.rs
+++ b/crates/ra_syntax/src/ast/make.rs
@@ -4,6 +4,10 @@ use itertools::Itertools;
 
 use crate::{ast, AstNode, SourceFile};
 
+pub fn name(text: &str) -> ast::Name {
+    ast_from_text(&format!("mod {};", text))
+}
+
 pub fn name_ref(text: &str) -> ast::NameRef {
     ast_from_text(&format!("fn f() {{ {}; }}", text))
 }
@@ -43,6 +47,21 @@ pub fn expr_unit() -> ast::Expr {
 pub fn expr_unimplemented() -> ast::Expr {
     expr_from_text("unimplemented!()")
 }
+pub fn expr_path(path: ast::Path) -> ast::Expr {
+    expr_from_text(&path.syntax().to_string())
+}
+pub fn expr_continue() -> ast::Expr {
+    expr_from_text("continue")
+}
+pub fn expr_break() -> ast::Expr {
+    expr_from_text("break")
+}
+pub fn expr_return() -> ast::Expr {
+    expr_from_text("return")
+}
+pub fn expr_match(expr: ast::Expr, match_arm_list: ast::MatchArmList) -> ast::Expr {
+    expr_from_text(&format!("match {} {}", expr.syntax(), match_arm_list.syntax()))
+}
 fn expr_from_text(text: &str) -> ast::Expr {
     ast_from_text(&format!("const C: () = {};", text))
 }
@@ -65,9 +84,9 @@ pub fn placeholder_pat() -> ast::PlaceholderPat {
 
 pub fn tuple_struct_pat(
     path: ast::Path,
-    pats: impl Iterator<Item = ast::Pat>,
+    pats: impl IntoIterator<Item = ast::Pat>,
 ) -> ast::TupleStructPat {
-    let pats_str = pats.map(|p| p.syntax().to_string()).join(", ");
+    let pats_str = pats.into_iter().map(|p| p.syntax().to_string()).join(", ");
     return from_text(&format!("{}({})", path.syntax(), pats_str));
 
     fn from_text(text: &str) -> ast::TupleStructPat {
@@ -75,8 +94,8 @@ pub fn tuple_struct_pat(
     }
 }
 
-pub fn record_pat(path: ast::Path, pats: impl Iterator<Item = ast::Pat>) -> ast::RecordPat {
-    let pats_str = pats.map(|p| p.syntax().to_string()).join(", ");
+pub fn record_pat(path: ast::Path, pats: impl IntoIterator<Item = ast::Pat>) -> ast::RecordPat {
+    let pats_str = pats.into_iter().map(|p| p.syntax().to_string()).join(", ");
     return from_text(&format!("{} {{ {} }}", path.syntax(), pats_str));
 
     fn from_text(text: &str) -> ast::RecordPat {
@@ -92,8 +111,8 @@ pub fn path_pat(path: ast::Path) -> ast::PathPat {
     }
 }
 
-pub fn match_arm(pats: impl Iterator<Item = ast::Pat>, expr: ast::Expr) -> ast::MatchArm {
-    let pats_str = pats.map(|p| p.syntax().to_string()).join(" | ");
+pub fn match_arm(pats: impl IntoIterator<Item = ast::Pat>, expr: ast::Expr) -> ast::MatchArm {
+    let pats_str = pats.into_iter().map(|p| p.syntax().to_string()).join(" | ");
     return from_text(&format!("{} => {}", pats_str, expr.syntax()));
 
     fn from_text(text: &str) -> ast::MatchArm {
@@ -101,8 +120,8 @@ pub fn match_arm(pats: impl Iterator<Item = ast::Pat>, expr: ast::Expr) -> ast::
     }
 }
 
-pub fn match_arm_list(arms: impl Iterator<Item = ast::MatchArm>) -> ast::MatchArmList {
-    let arms_str = arms.map(|arm| format!("\n    {}", arm.syntax())).join(",");
+pub fn match_arm_list(arms: impl IntoIterator<Item = ast::MatchArm>) -> ast::MatchArmList {
+    let arms_str = arms.into_iter().map(|arm| format!("\n    {}", arm.syntax())).join(",");
     return from_text(&format!("{},\n", arms_str));
 
     fn from_text(text: &str) -> ast::MatchArmList {
@@ -110,25 +129,11 @@ pub fn match_arm_list(arms: impl Iterator<Item = ast::MatchArm>) -> ast::MatchAr
     }
 }
 
-pub fn let_match_early(expr: ast::Expr, path: &str, early_expression: &str) -> ast::LetStmt {
-    return from_text(&format!(
-        r#"let {} = match {} {{
-    {}(it) => it,
-    None => {},
-}};"#,
-        expr.syntax().text(),
-        expr.syntax().text(),
-        path,
-        early_expression
-    ));
-
-    fn from_text(text: &str) -> ast::LetStmt {
-        ast_from_text(&format!("fn f() {{ {} }}", text))
-    }
-}
-
-pub fn where_pred(path: ast::Path, bounds: impl Iterator<Item = ast::TypeBound>) -> ast::WherePred {
-    let bounds = bounds.map(|b| b.syntax().to_string()).join(" + ");
+pub fn where_pred(
+    path: ast::Path,
+    bounds: impl IntoIterator<Item = ast::TypeBound>,
+) -> ast::WherePred {
+    let bounds = bounds.into_iter().map(|b| b.syntax().to_string()).join(" + ");
     return from_text(&format!("{}: {}", path.syntax(), bounds));
 
     fn from_text(text: &str) -> ast::WherePred {
@@ -136,8 +141,8 @@ pub fn where_pred(path: ast::Path, bounds: impl Iterator<Item = ast::TypeBound>)
     }
 }
 
-pub fn where_clause(preds: impl Iterator<Item = ast::WherePred>) -> ast::WhereClause {
-    let preds = preds.map(|p| p.syntax().to_string()).join(", ");
+pub fn where_clause(preds: impl IntoIterator<Item = ast::WherePred>) -> ast::WhereClause {
+    let preds = preds.into_iter().map(|p| p.syntax().to_string()).join(", ");
     return from_text(preds.as_str());
 
     fn from_text(text: &str) -> ast::WhereClause {
@@ -153,6 +158,14 @@ pub fn if_expression(condition: &ast::Expr, statement: &str) -> ast::IfExpr {
     ))
 }
 
+pub fn let_stmt(pattern: ast::Pat, initializer: Option<ast::Expr>) -> ast::LetStmt {
+    let text = match initializer {
+        Some(it) => format!("let {} = {};", pattern.syntax(), it.syntax()),
+        None => format!("let {};", pattern.syntax()),
+    };
+    ast_from_text(&format!("fn f() {{ {} }}", text))
+}
+
 fn ast_from_text<N: AstNode>(text: &str) -> N {
     let parse = SourceFile::parse(text);
     let res = parse.tree().syntax().descendants().find_map(N::cast).unwrap();
diff --git a/crates/ra_syntax/src/syntax_error.rs b/crates/ra_syntax/src/syntax_error.rs
index d6eca2ad7..1f60a7aab 100644
--- a/crates/ra_syntax/src/syntax_error.rs
+++ b/crates/ra_syntax/src/syntax_error.rs
@@ -82,6 +82,7 @@ pub enum SyntaxErrorKind {
     InvalidBlockAttr,
     InvalidMatchInnerAttr,
     InvalidTupleIndexFormat,
+    VisibilityNotAllowed,
 }
 
 impl fmt::Display for SyntaxErrorKind {
@@ -99,6 +100,9 @@ impl fmt::Display for SyntaxErrorKind {
             }
             ParseError(msg) => write!(f, "{}", msg.0),
             EscapeError(err) => write!(f, "{}", err),
+            VisibilityNotAllowed => {
+                write!(f, "unnecessary visibility qualifier")
+            }
         }
     }
 }
diff --git a/crates/ra_syntax/src/validation.rs b/crates/ra_syntax/src/validation.rs
index ab4f15908..2d596763e 100644
--- a/crates/ra_syntax/src/validation.rs
+++ b/crates/ra_syntax/src/validation.rs
@@ -6,7 +6,7 @@ use rustc_lexer::unescape;
 
 use crate::{
     ast, match_ast, AstNode, SyntaxError, SyntaxErrorKind,
-    SyntaxKind::{BYTE, BYTE_STRING, CHAR, INT_NUMBER, STRING},
+    SyntaxKind::{BYTE, BYTE_STRING, CHAR, CONST_DEF, FN_DEF, INT_NUMBER, STRING, TYPE_ALIAS_DEF},
     SyntaxNode, SyntaxToken, TextUnit, T,
 };
 
@@ -102,6 +102,7 @@ pub(crate) fn validate(root: &SyntaxNode) -> Vec<SyntaxError> {
                 ast::BlockExpr(it) => { block::validate_block_expr(it, &mut errors) },
                 ast::FieldExpr(it) => { validate_numeric_name(it.name_ref(), &mut errors) },
                 ast::RecordField(it) => { validate_numeric_name(it.name_ref(), &mut errors) },
+                ast::Visibility(it) => { validate_visibility(it, &mut errors) },
                 _ => (),
             }
         }
@@ -206,3 +207,23 @@ fn validate_numeric_name(name_ref: Option<ast::NameRef>, errors: &mut Vec<Syntax
         name_ref?.syntax().first_child_or_token()?.into_token().filter(|it| it.kind() == INT_NUMBER)
     }
 }
+
+fn validate_visibility(vis: ast::Visibility, errors: &mut Vec<SyntaxError>) {
+    let parent = match vis.syntax().parent() {
+        Some(it) => it,
+        None => return,
+    };
+    match parent.kind() {
+        FN_DEF | CONST_DEF | TYPE_ALIAS_DEF => (),
+        _ => return,
+    }
+    let impl_block = match parent.parent().and_then(|it| it.parent()).and_then(ast::ImplBlock::cast)
+    {
+        Some(it) => it,
+        None => return,
+    };
+    if impl_block.target_trait().is_some() {
+        errors
+            .push(SyntaxError::new(SyntaxErrorKind::VisibilityNotAllowed, vis.syntax.text_range()))
+    }
+}