Skip to content

Commit d0a141e

Browse files
WohopsWohops
committed
Fix quality flaw: remove useless methods duplicating fields
1 parent a77456f commit d0a141e

5 files changed

Lines changed: 43 additions & 70 deletions

File tree

‎java-squid/src/main/java/org/sonar/java/resolve/FirstPass.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -188,9 +188,9 @@ public void visitIdentifier(IdentifierTree tree) {
188188
if (JavaPunctuator.STAR.getValue().equals(tree.name())) {
189189
//star import : we save the current symbol
190190
if (isStatic) {
191-
env.staticStarImports().enter(currentSymbol);
191+
env.staticStarImports.enter(currentSymbol);
192192
} else {
193-
env.starImports().enter(currentSymbol);
193+
env.starImports.enter(currentSymbol);
194194
}
195195
//we set current symbol to not found to do not put it in named import scope.
196196
currentSymbol = new Resolve.JavaSymbolNotFound();

‎java-squid/src/main/java/org/sonar/java/resolve/JavaSymbol.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -270,7 +270,7 @@ public PackageJavaSymbol(@Nullable String name, @Nullable JavaSymbol owner) {
270270
super(PCK, 0, name, owner);
271271
}
272272

273-
Scope members() {
273+
Scope completedMembers() {
274274
complete();
275275
return members;
276276
}

‎java-squid/src/main/java/org/sonar/java/resolve/Resolve.java‎

Lines changed: 24 additions & 50 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@
2323
import com.google.common.collect.ImmutableList;
2424

2525
import javax.annotation.Nullable;
26+
2627
import java.util.List;
2728
import java.util.Map;
2829

@@ -134,23 +135,23 @@ private Resolution findVar(Env env, String name) {
134135
Resolution bestSoFar = unresolved();
135136

136137
Env env1 = env;
137-
while (env1.outer() != null) {
138+
while (env1.outer != null) {
138139
Resolution sym = new Resolution();
139-
for (JavaSymbol symbol : env1.scope().lookup(name)) {
140+
for (JavaSymbol symbol : env1.scope.lookup(name)) {
140141
if (symbol.kind == JavaSymbol.VAR) {
141142
sym.symbol = symbol;
142143
}
143144
}
144145
if (sym.symbol == null) {
145-
sym = findField(env1, env1.enclosingClass(), name, env1.enclosingClass());
146+
sym = findField(env1, env1.enclosingClass, name, env1.enclosingClass);
146147
}
147148
if (sym.symbol.kind < JavaSymbol.ERRONEOUS) {
148149
// symbol exists
149150
return sym;
150151
} else if (sym.symbol.kind < bestSoFar.symbol.kind) {
151152
bestSoFar = sym;
152153
}
153-
env1 = env1.outer();
154+
env1 = env1.outer;
154155
}
155156

156157
JavaSymbol symbol = findInStaticImport(env, name, JavaSymbol.VAR);
@@ -170,12 +171,12 @@ private JavaSymbol findInStaticImport(Env env, String name, int kind) {
170171
JavaSymbol bestSoFar = symbolNotFound;
171172
//imports
172173
//Ok because clash of name between type and var/method result in compile error: JLS8 7.5.3
173-
for (JavaSymbol symbol : env.namedImports().lookup(name)) {
174+
for (JavaSymbol symbol : env.namedImports.lookup(name)) {
174175
if ((kind & symbol.kind) != 0) {
175176
return symbol;
176177
}
177178
}
178-
for (JavaSymbol symbol : env.staticStarImports().lookup(name)) {
179+
for (JavaSymbol symbol : env.staticStarImports.lookup(name)) {
179180
if ((kind & symbol.kind) != 0) {
180181
return symbol;
181182
}
@@ -212,14 +213,14 @@ private JavaSymbol findMemberType(Env env, JavaSymbol.TypeJavaSymbol site, Strin
212213
*/
213214
private JavaSymbol findType(Env env, String name) {
214215
JavaSymbol bestSoFar = symbolNotFound;
215-
for (Env env1 = env; env1 != null; env1 = env1.outer()) {
216-
for (JavaSymbol symbol : env1.scope().lookup(name)) {
216+
for (Env env1 = env; env1 != null; env1 = env1.outer) {
217+
for (JavaSymbol symbol : env1.scope.lookup(name)) {
217218
if (symbol.kind == JavaSymbol.TYP) {
218219
return symbol;
219220
}
220221
}
221222
if (env1.outer != null) {
222-
JavaSymbol symbol = findMemberType(env1, env1.enclosingClass(), name, env1.enclosingClass());
223+
JavaSymbol symbol = findMemberType(env1, env1.enclosingClass, name, env1.enclosingClass);
223224
if (symbol.kind < JavaSymbol.ERRONEOUS) {
224225
// symbol exists
225226
return symbol;
@@ -237,25 +238,25 @@ private JavaSymbol findType(Env env, String name) {
237238

238239
//JLS8 6.4.1 Shadowing rules
239240
//named imports
240-
for (JavaSymbol symbol : env.namedImports().lookup(name)) {
241+
for (JavaSymbol symbol : env.namedImports.lookup(name)) {
241242
if (symbol.kind == JavaSymbol.TYP) {
242243
return symbol;
243244
}
244245
}
245246
//package types
246-
JavaSymbol sym = findIdentInPackage(env.packge(), name, JavaSymbol.TYP);
247+
JavaSymbol sym = findIdentInPackage(env.packge, name, JavaSymbol.TYP);
247248
if (sym.kind < bestSoFar.kind) {
248249
return sym;
249250
}
250251
//on demand imports
251-
for (JavaSymbol symbol : env.starImports().lookup(name)) {
252+
for (JavaSymbol symbol : env.starImports.lookup(name)) {
252253
if (symbol.kind == JavaSymbol.TYP) {
253254
return symbol;
254255
}
255256
}
256257
//java.lang
257258
JavaSymbol.PackageJavaSymbol javaLang = bytecodeCompleter.enterPackage("java.lang");
258-
for (JavaSymbol symbol : javaLang.members().lookup(name)) {
259+
for (JavaSymbol symbol : javaLang.completedMembers().lookup(name)) {
259260
if (symbol.kind == JavaSymbol.TYP) {
260261
return symbol;
261262
}
@@ -354,8 +355,8 @@ public Resolution findIdentInType(Env env, JavaSymbol.TypeJavaSymbol site, Strin
354355
public Resolution findMethod(Env env, String name, List<JavaType> argTypes, List<JavaType> typeParamTypes) {
355356
Resolution bestSoFar = unresolved();
356357
Env env1 = env;
357-
while (env1.outer() != null) {
358-
Resolution res = findMethod(env1, env1.enclosingClass().getType(), name, argTypes, typeParamTypes);
358+
while (env1.outer != null) {
359+
Resolution res = findMethod(env1, env1.enclosingClass.getType(), name, argTypes, typeParamTypes);
359360
if (res.symbol.kind < JavaSymbol.ERRONEOUS) {
360361
// symbol exists
361362
return res;
@@ -541,16 +542,16 @@ boolean isAccessible(Env env, JavaSymbol.TypeJavaSymbol c) {
541542
final boolean result;
542543
switch (c.flags() & Flags.ACCESS_FLAGS) {
543544
case Flags.PRIVATE:
544-
result = env.enclosingClass().outermostClass() == c.owner().outermostClass();
545+
result = env.enclosingClass.outermostClass() == c.owner().outermostClass();
545546
break;
546547
case 0:
547-
result = env.packge() == c.packge();
548+
result = env.packge == c.packge();
548549
break;
549550
case Flags.PUBLIC:
550551
result = true;
551552
break;
552553
case Flags.PROTECTED:
553-
result = env.packge() == c.packge() || isInnerSubClass(env.enclosingClass(), c.owner());
554+
result = env.packge == c.packge() || isInnerSubClass(env.enclosingClass, c.owner());
554555
break;
555556
default:
556557
throw new IllegalStateException();
@@ -607,18 +608,18 @@ private boolean isAccessible(Env env, JavaSymbol.TypeJavaSymbol site, JavaSymbol
607608
case Flags.PRIVATE:
608609
//if enclosing class is null, we are checking accessibility for imports so we return false.
609610
// no check of overriding, because private members cannot be overridden
610-
return env.enclosingClass != null && (env.enclosingClass().outermostClass() == symbol.owner().outermostClass())
611+
return env.enclosingClass != null && (env.enclosingClass.outermostClass() == symbol.owner().outermostClass())
611612
&& isInheritedIn(symbol, site);
612613
case 0:
613-
return (env.packge() == symbol.packge())
614+
return (env.packge == symbol.packge())
614615
&& isAccessible(env, site)
615616
&& isInheritedIn(symbol, site)
616617
&& notOverriddenIn(site, symbol);
617618
case Flags.PUBLIC:
618619
return isAccessible(env, site)
619620
&& notOverriddenIn(site, symbol);
620621
case Flags.PROTECTED:
621-
return ((env.packge() == symbol.packge()) || isProtectedAccessible(symbol, env.enclosingClass, site))
622+
return ((env.packge == symbol.packge()) || isProtectedAccessible(symbol, env.enclosingClass, site))
622623
&& isAccessible(env, site)
623624
&& notOverriddenIn(site, symbol);
624625
default:
@@ -716,46 +717,19 @@ static class Env {
716717
/**
717718
* The environment enclosing the current class.
718719
*/
720+
@Nullable
719721
Env outer;
720722

721723
JavaSymbol.PackageJavaSymbol packge;
722724

725+
@Nullable
723726
JavaSymbol.TypeJavaSymbol enclosingClass;
724727

725728
Scope scope;
726729
Scope namedImports;
727730
Scope starImports;
728731
Scope staticStarImports;
729732

730-
@Nullable
731-
Env outer() {
732-
return outer;
733-
}
734-
735-
JavaSymbol.TypeJavaSymbol enclosingClass() {
736-
return enclosingClass;
737-
}
738-
739-
public JavaSymbol.PackageJavaSymbol packge() {
740-
return packge;
741-
}
742-
743-
Scope namedImports() {
744-
return namedImports;
745-
}
746-
747-
Scope starImports() {
748-
return starImports;
749-
}
750-
751-
public Scope staticStarImports() {
752-
return staticStarImports;
753-
}
754-
755-
Scope scope() {
756-
return scope;
757-
}
758-
759733
public Env dup() {
760734
Env env = new Env();
761735
env.next = this;

‎java-squid/src/main/java/org/sonar/java/resolve/SecondPass.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -205,7 +205,7 @@ private void complete(JavaSymbol.VariableJavaSymbol symbol) {
205205
VariableTree variableTree = symbol.declaration;
206206
Resolve.Env env = semanticModel.getEnv(symbol);
207207
if (variableTree.is(Tree.Kind.ENUM_CONSTANT)) {
208-
symbol.type = env.enclosingClass().type;
208+
symbol.type = env.enclosingClass.type;
209209
} else {
210210
symbol.type = resolveType(env, variableTree.type());
211211
}

‎java-squid/src/test/java/org/sonar/java/resolve/ResolveTest.java‎

Lines changed: 15 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -21,13 +21,13 @@
2121

2222
import com.google.common.collect.ImmutableList;
2323
import com.google.common.collect.Lists;
24+
import org.junit.Before;
2425
import org.junit.Test;
2526

2627
import java.io.File;
2728

2829
import static org.fest.assertions.Assertions.assertThat;
2930
import static org.mockito.Mockito.mock;
30-
import static org.mockito.Mockito.when;
3131

3232
public class ResolveTest {
3333

@@ -37,22 +37,25 @@ public class ResolveTest {
3737

3838
private Resolve.Env env = mock(Resolve.Env.class);
3939

40+
@Before
41+
public void setUp() {
42+
env = new Resolve.Env();
43+
env.packge = new JavaSymbol.PackageJavaSymbol(null, null);
44+
}
45+
4046
@Test
4147
public void access_public_class() {
42-
JavaSymbol.PackageJavaSymbol packageSymbol = new JavaSymbol.PackageJavaSymbol(null, null);
43-
JavaSymbol.TypeJavaSymbol targetClassSymbol = new JavaSymbol.TypeJavaSymbol(Flags.PUBLIC, "TargetClass", packageSymbol);
48+
JavaSymbol.TypeJavaSymbol targetClassSymbol = new JavaSymbol.TypeJavaSymbol(Flags.PUBLIC, "TargetClass", env.packge);
4449
assertThat(resolve.isAccessible(env, targetClassSymbol)).isTrue();
4550
}
4651

4752
@Test
4853
public void access_protected_class() {
49-
JavaSymbol.PackageJavaSymbol packageSymbol = new JavaSymbol.PackageJavaSymbol(null, null);
50-
JavaSymbol.TypeJavaSymbol targetClassSymbol = new JavaSymbol.TypeJavaSymbol(Flags.PROTECTED, "TargetClass", packageSymbol);
54+
JavaSymbol.TypeJavaSymbol targetClassSymbol = new JavaSymbol.TypeJavaSymbol(Flags.PROTECTED, "TargetClass", env.packge);
5155

52-
when(env.packge()).thenReturn(packageSymbol);
5356
assertThat(resolve.isAccessible(env, targetClassSymbol)).isTrue();
5457

55-
when(env.packge()).thenReturn(new JavaSymbol.PackageJavaSymbol("AnotherPackage", null));
58+
env.packge = new JavaSymbol.PackageJavaSymbol("AnotherPackage", null);
5659
assertThat(resolve.isAccessible(env, targetClassSymbol)).isFalse();
5760
}
5861

@@ -68,13 +71,11 @@ public void access_protected_class() {
6871
*/
6972
@Test
7073
public void access_package_local_class() {
71-
JavaSymbol.PackageJavaSymbol packageSymbol = new JavaSymbol.PackageJavaSymbol(null, null);
72-
JavaSymbol.TypeJavaSymbol targetClassSymbol = new JavaSymbol.TypeJavaSymbol(0, "TargetClass", packageSymbol);
74+
JavaSymbol.TypeJavaSymbol targetClassSymbol = new JavaSymbol.TypeJavaSymbol(0, "TargetClass", env.packge);
7375

74-
when(env.packge()).thenReturn(packageSymbol);
7576
assertThat(resolve.isAccessible(env, targetClassSymbol)).isTrue();
7677

77-
when(env.packge()).thenReturn(new JavaSymbol.PackageJavaSymbol("AnotherPackage", null));
78+
env.packge = new JavaSymbol.PackageJavaSymbol("AnotherPackage", null);
7879
assertThat(resolve.isAccessible(env, targetClassSymbol)).isFalse();
7980
}
8081

@@ -92,14 +93,12 @@ public void access_package_local_class() {
9293
*/
9394
@Test
9495
public void access_private_class() {
95-
JavaSymbol.PackageJavaSymbol packageSymbol = new JavaSymbol.PackageJavaSymbol(null, null);
96-
JavaSymbol.TypeJavaSymbol outermostClassSymbol = new JavaSymbol.TypeJavaSymbol(0, "OutermostClass", packageSymbol);
97-
JavaSymbol.TypeJavaSymbol targetClassSymbol = new JavaSymbol.TypeJavaSymbol(Flags.PRIVATE, "TargetClass", outermostClassSymbol);
96+
env.enclosingClass = new JavaSymbol.TypeJavaSymbol(0, "OutermostClass", env.packge);
97+
JavaSymbol.TypeJavaSymbol targetClassSymbol = new JavaSymbol.TypeJavaSymbol(Flags.PRIVATE, "TargetClass", env.enclosingClass);
9898

99-
when(env.enclosingClass()).thenReturn(outermostClassSymbol);
10099
assertThat(resolve.isAccessible(env, targetClassSymbol)).isTrue();
101100

102-
when(env.enclosingClass()).thenReturn(new JavaSymbol.TypeJavaSymbol(0, "AnotherOutermostClass", packageSymbol));
101+
env.enclosingClass = new JavaSymbol.TypeJavaSymbol(0, "AnotherOutermostClass", env.packge);
103102
assertThat(resolve.isAccessible(env, targetClassSymbol)).isFalse();
104103
}
105104

0 commit comments

Comments
 (0)