Merge #2207

2207: Forbid visibility qualifiers in traits r=matklad a=matklad



Co-authored-by: Aleksey Kladov <[email protected]>

4 files changed, 131 insertions, 1 deletions

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()))
+    }
+}
diff --git a/crates/ra_syntax/test_data/parser/err/0037_visibility_in_traits.rs b/crates/ra_syntax/test_data/parser/err/0037_visibility_in_traits.rs
new file mode 100644
index 000000000..a43e7ef10
--- /dev/null
+++ b/crates/ra_syntax/test_data/parser/err/0037_visibility_in_traits.rs
@@ -0,0 +1,6 @@
+impl T for () {
+    fn foo() {}
+    pub fn bar() {}
+    pub(crate) type Baz = ();
+    pub(crate) const C: i32 = 92;
+}
diff --git a/crates/ra_syntax/test_data/parser/err/0037_visibility_in_traits.txt b/crates/ra_syntax/test_data/parser/err/0037_visibility_in_traits.txt
new file mode 100644
index 000000000..749c8cddb
--- /dev/null
+++ b/crates/ra_syntax/test_data/parser/err/0037_visibility_in_traits.txt
@@ -0,0 +1,99 @@
+SOURCE_FILE@[0; 118)
+  IMPL_BLOCK@[0; 117)
+    IMPL_KW@[0; 4) "impl"
+    WHITESPACE@[4; 5) " "
+    PATH_TYPE@[5; 6)
+      PATH@[5; 6)
+        PATH_SEGMENT@[5; 6)
+          NAME_REF@[5; 6)
+            IDENT@[5; 6) "T"
+    WHITESPACE@[6; 7) " "
+    FOR_KW@[7; 10) "for"
+    WHITESPACE@[10; 11) " "
+    TUPLE_TYPE@[11; 13)
+      L_PAREN@[11; 12) "("
+      R_PAREN@[12; 13) ")"
+    WHITESPACE@[13; 14) " "
+    ITEM_LIST@[14; 117)
+      L_CURLY@[14; 15) "{"
+      WHITESPACE@[15; 20) "\n    "
+      FN_DEF@[20; 31)
+        FN_KW@[20; 22) "fn"
+        WHITESPACE@[22; 23) " "
+        NAME@[23; 26)
+          IDENT@[23; 26) "foo"
+        PARAM_LIST@[26; 28)
+          L_PAREN@[26; 27) "("
+          R_PAREN@[27; 28) ")"
+        WHITESPACE@[28; 29) " "
+        BLOCK_EXPR@[29; 31)
+          BLOCK@[29; 31)
+            L_CURLY@[29; 30) "{"
+            R_CURLY@[30; 31) "}"
+      WHITESPACE@[31; 36) "\n    "
+      FN_DEF@[36; 51)
+        VISIBILITY@[36; 39)
+          PUB_KW@[36; 39) "pub"
+        WHITESPACE@[39; 40) " "
+        FN_KW@[40; 42) "fn"
+        WHITESPACE@[42; 43) " "
+        NAME@[43; 46)
+          IDENT@[43; 46) "bar"
+        PARAM_LIST@[46; 48)
+          L_PAREN@[46; 47) "("
+          R_PAREN@[47; 48) ")"
+        WHITESPACE@[48; 49) " "
+        BLOCK_EXPR@[49; 51)
+          BLOCK@[49; 51)
+            L_CURLY@[49; 50) "{"
+            R_CURLY@[50; 51) "}"
+      WHITESPACE@[51; 56) "\n    "
+      TYPE_ALIAS_DEF@[56; 81)
+        VISIBILITY@[56; 66)
+          PUB_KW@[56; 59) "pub"
+          L_PAREN@[59; 60) "("
+          CRATE_KW@[60; 65) "crate"
+          R_PAREN@[65; 66) ")"
+        WHITESPACE@[66; 67) " "
+        TYPE_KW@[67; 71) "type"
+        WHITESPACE@[71; 72) " "
+        NAME@[72; 75)
+          IDENT@[72; 75) "Baz"
+        WHITESPACE@[75; 76) " "
+        EQ@[76; 77) "="
+        WHITESPACE@[77; 78) " "
+        TUPLE_TYPE@[78; 80)
+          L_PAREN@[78; 79) "("
+          R_PAREN@[79; 80) ")"
+        SEMI@[80; 81) ";"
+      WHITESPACE@[81; 86) "\n    "
+      CONST_DEF@[86; 115)
+        VISIBILITY@[86; 96)
+          PUB_KW@[86; 89) "pub"
+          L_PAREN@[89; 90) "("
+          CRATE_KW@[90; 95) "crate"
+          R_PAREN@[95; 96) ")"
+        WHITESPACE@[96; 97) " "
+        CONST_KW@[97; 102) "const"
+        WHITESPACE@[102; 103) " "
+        NAME@[103; 104)
+          IDENT@[103; 104) "C"
+        COLON@[104; 105) ":"
+        WHITESPACE@[105; 106) " "
+        PATH_TYPE@[106; 109)
+          PATH@[106; 109)
+            PATH_SEGMENT@[106; 109)
+              NAME_REF@[106; 109)
+                IDENT@[106; 109) "i32"
+        WHITESPACE@[109; 110) " "
+        EQ@[110; 111) "="
+        WHITESPACE@[111; 112) " "
+        LITERAL@[112; 114)
+          INT_NUMBER@[112; 114) "92"
+        SEMI@[114; 115) ";"
+      WHITESPACE@[115; 116) "\n"
+      R_CURLY@[116; 117) "}"
+  WHITESPACE@[117; 118) "\n"
+error [36; 39): unnecessary visibility qualifier
+error [56; 66): unnecessary visibility qualifier
+error [86; 96): unnecessary visibility qualifier