You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@lucenenet.apache.org by GitBox <gi...@apache.org> on 2022/10/29 10:41:40 UTC
[GitHub] [lucenenet] sachdevlaksh opened a new pull request, #729: #679 Added sonar suggestions
sachdevlaksh opened a new pull request, #729:
URL: https://github.com/apache/lucenenet/pull/729
Fixes #679 related to code quality issue detected by Sonar(access modifier to reduce visibility)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@lucenenet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [lucenenet] NightOwl888 commented on a diff in pull request #729: #679 Added sonar suggestions
Posted by GitBox <gi...@apache.org>.
NightOwl888 commented on code in PR #729:
URL: https://github.com/apache/lucenenet/pull/729#discussion_r1008961473
##########
src/Lucene.Net.Analysis.Common/Analysis/En/KStemData1.cs:
##########
@@ -49,7 +49,7 @@ private KStemData1()
{
}
// KStemData1 ... KStemData8 are created from "head_word_list.txt"
- internal static string[] data = new string[] {
+ internal static readonly string[] data = new string[] {
Review Comment:
Please add a comment to indicate we have intentionally deviated from [Lucene 4.8.1](https://github.com/apache/lucene/blob/releases/lucene-solr/4.8.1/lucene/analysis/common/src/java/org/apache/lucene/analysis/en/KStemData1.java#L48) on this line.
`// LUCENENET: Marked readonly`
##########
src/Lucene.Net.Analysis.Common/Analysis/En/KStemData3.cs:
##########
@@ -47,7 +47,7 @@ internal class KStemData3
private KStemData3()
{
}
- internal static string[] data = new string[] {
+ internal static readonly string[] data = new string[] {
Review Comment:
Please add a comment to indicate we have intentionally deviated from [Lucene 4.8.1](https://github.com/apache/lucene/blob/releases/lucene-solr/4.8.1/lucene/analysis/common/src/java/org/apache/lucene/analysis/en/KStemData3.java#L46) on this line.
`// LUCENENET: Marked readonly`
##########
src/Lucene.Net.Analysis.Common/Analysis/En/KStemData7.cs:
##########
@@ -47,7 +47,7 @@ internal class KStemData7
private KStemData7()
{
}
- internal static string[] data = new string[] {
+ internal static readonly string[] data = new string[] {
Review Comment:
Please add a comment to indicate we have intentionally deviated from [Lucene 4.8.1](https://github.com/apache/lucene/blob/releases/lucene-solr/4.8.1/lucene/analysis/common/src/java/org/apache/lucene/analysis/en/KStemData7.java#L46) on this line.
`// LUCENENET: Marked readonly`
##########
src/Lucene.Net.QueryParser/Flexible/Standard/Parser/StandardSyntaxParser.cs:
##########
@@ -1253,7 +1253,7 @@ public virtual ParseException GenerateParseException()
{
exptokseq[i] = jj_expentries[i];
}
- return new ParseException(Token, exptokseq, StandardSyntaxParserConstants.TokenImage);
+ return new ParseException(Token, exptokseq, StandardSyntaxParserConstants.TOKEN_IMAGE);
Review Comment:
Please revert. See the comment in Classic QueryParser.
##########
src/Lucene.Net.Suggest/Suggest/Analyzing/BlendedInfixSuggester.cs:
##########
@@ -47,12 +47,12 @@ public class BlendedInfixSuggester : AnalyzingInfixSuggester
/// <summary>
/// Coefficient used for linear blending
/// </summary>
- protected internal static double LINEAR_COEF = 0.10;
+ private static readonly double LINEAR_COEF = 0.10;
Review Comment:
Let's revert this. Can't be sure whether or not this was intentionally supposed to be a user setting, but it is the same declaration as [Lucene 4.8.1](https://github.com/apache/lucene/blob/releases/lucene-solr/4.8.1/lucene/suggest/src/java/org/apache/lucene/search/suggest/analyzing/BlendedInfixSuggester.java#L61).
Similarly, please revert `DEFAULT_NUM_FACTOR`.
Please suppress both of these using the following attribute.
`[SuppressMessage("Usage", "CA2211:Non-constant fields should not be visible", Justification = "Lucene may have intended for this to be a user setting")]`
##########
src/Lucene.Net.Analysis.Common/Analysis/En/KStemData2.cs:
##########
@@ -47,7 +47,7 @@ internal class KStemData2
private KStemData2()
{
}
- internal static string[] data = new string[] {
+ internal static readonly string[] data = new string[] {
Review Comment:
Please add a comment to indicate we have intentionally deviated from [Lucene 4.8.1](https://github.com/apache/lucene/blob/releases/lucene-solr/4.8.1/lucene/analysis/common/src/java/org/apache/lucene/analysis/en/KStemData2.java#L46) on this line.
`// LUCENENET: Marked readonly`
##########
src/Lucene.Net/Util/Packed/PackedInts.cs:
##########
@@ -46,32 +46,32 @@ public static class PackedInt32s // LUCENENET specific: CA1052 Static holder typ
/// <summary>
/// At most 700% memory overhead, always select a direct implementation.
/// </summary>
- public static float FASTEST = 7f;
+ public static readonly float FASTEST = 7f;
/// <summary>
/// At most 50% memory overhead, always select a reasonably fast implementation.
/// </summary>
- public static float FAST = 0.5f;
+ public static readonly float FAST = 0.5f;
Review Comment:
Will accept this because it was this way in [Lucene 4.8.0](https://github.com/apache/lucene/blob/releases/lucene-solr/4.8.0/lucene/core/src/java/org/apache/lucene/util/packed/PackedInts.java#L49), but do note that this was supposed to be part of #662 (it should be const).
##########
src/Lucene.Net.Analysis.Common/Analysis/En/KStemData8.cs:
##########
@@ -47,7 +47,7 @@ internal class KStemData8
private KStemData8()
{
}
- internal static string[] data = new string[] {
+ internal static readonly string[] data = new string[] {
Review Comment:
Please add a comment to indicate we have intentionally deviated from [Lucene 4.8.1](https://github.com/apache/lucene/blob/releases/lucene-solr/4.8.1/lucene/analysis/common/src/java/org/apache/lucene/analysis/en/KStemData8.java#L46) on this line.
`// LUCENENET: Marked readonly`
##########
src/Lucene.Net.Analysis.Common/Analysis/En/KStemData4.cs:
##########
@@ -47,7 +47,7 @@ internal class KStemData4
private KStemData4()
{
}
- internal static string[] data = new string[] {
+ internal static readonly string[] data = new string[] {
Review Comment:
Please add a comment to indicate we have intentionally deviated from [Lucene 4.8.1](https://github.com/apache/lucene/blob/releases/lucene-solr/4.8.1/lucene/analysis/common/src/java/org/apache/lucene/analysis/en/KStemData4.java#L46) on this line.
`// LUCENENET: Marked readonly`
##########
src/Lucene.Net.Benchmark/Constants.cs:
##########
@@ -26,7 +26,7 @@ public static class Constants // LUCENENET specific: CA1052 Static holder types
public const int DEFAULT_SCALE_UP = 5;
public const int DEFAULT_LOG_STEP = 1000;
- public static bool[] BOOLEANS = new bool[] { false, true };
+ public static readonly bool[] BOOLEANS = new bool[] { false, true };
Review Comment:
Please add a comment to indicate we have intentionally deviated from [Lucene 4.8.1](https://github.com/apache/lucene/blob/releases/lucene-solr/4.8.1/lucene/benchmark/src/java/org/apache/lucene/benchmark/Constants.java#L29) on this line.
`// LUCENENET: Marked readonly`
##########
src/Lucene.Net.QueryParser/Classic/ParseException.cs:
##########
@@ -193,11 +193,11 @@ public override void GetObjectData(SerializationInfo info, StreamingContext cont
retval += expected.ToString();
return retval;
}
-
+
/// <summary>
/// The end of line string for this machine.
/// </summary>
- protected static string eol = Environment.NewLine;
+ protected static readonly string eol = Environment.NewLine;
Review Comment:
Please add a comment to indicate we have intentionally deviated from [Lucene 4.8.1](https://github.com/apache/lucene/blob/releases/lucene-solr/4.8.1/lucene/queryparser/src/java/org/apache/lucene/queryparser/classic/ParseException.java#L134) on this line.
`// LUCENENET: Marked readonly`
##########
src/Lucene.Net.Analysis.Common/Analysis/En/KStemData6.cs:
##########
@@ -47,7 +47,7 @@ internal class KStemData6
private KStemData6()
{
}
- internal static string[] data = new string[] {
+ internal static readonly string[] data = new string[] {
Review Comment:
Please add a comment to indicate we have intentionally deviated from [Lucene 4.8.1](https://github.com/apache/lucene/blob/releases/lucene-solr/4.8.1/lucene/analysis/common/src/java/org/apache/lucene/analysis/en/KStemData6.java#L46) on this line.
`// LUCENENET: Marked readonly`
##########
src/Lucene.Net.Analysis.Common/Analysis/En/KStemData5.cs:
##########
@@ -47,7 +47,7 @@ internal class KStemData5
private KStemData5()
{
}
- internal static string[] data = new string[] {
+ internal static readonly string[] data = new string[] {
Review Comment:
Please add a comment to indicate we have intentionally deviated from [Lucene 4.8.1](https://github.com/apache/lucene/blob/releases/lucene-solr/4.8.1/lucene/analysis/common/src/java/org/apache/lucene/analysis/en/KStemData5.java#L46) on this line.
`// LUCENENET: Marked readonly`
##########
src/Lucene.Net/Util/Packed/PackedInts.cs:
##########
@@ -46,32 +46,32 @@ public static class PackedInt32s // LUCENENET specific: CA1052 Static holder typ
/// <summary>
/// At most 700% memory overhead, always select a direct implementation.
/// </summary>
- public static float FASTEST = 7f;
+ public static readonly float FASTEST = 7f;
/// <summary>
/// At most 50% memory overhead, always select a reasonably fast implementation.
/// </summary>
- public static float FAST = 0.5f;
+ public static readonly float FAST = 0.5f;
/// <summary>
/// At most 20% memory overhead.
/// </summary>
- public static float DEFAULT = 0.2f;
+ public static readonly float DEFAULT = 0.2f;
/// <summary>
/// No memory overhead at all, but the returned implementation may be slow.
/// </summary>
- public static float COMPACT = 0f;
+ public static readonly float COMPACT = 0f;
Review Comment:
Will accept this because it was this way in [Lucene 4.8.0](https://github.com/apache/lucene/blob/releases/lucene-solr/4.8.0/lucene/core/src/java/org/apache/lucene/util/packed/PackedInts.java#L59), but do note that this was supposed to be part of #662 (it should be const).
##########
src/Lucene.Net/Util/Packed/PackedInts.cs:
##########
@@ -46,32 +46,32 @@ public static class PackedInt32s // LUCENENET specific: CA1052 Static holder typ
/// <summary>
/// At most 700% memory overhead, always select a direct implementation.
/// </summary>
- public static float FASTEST = 7f;
+ public static readonly float FASTEST = 7f;
/// <summary>
/// At most 50% memory overhead, always select a reasonably fast implementation.
/// </summary>
- public static float FAST = 0.5f;
+ public static readonly float FAST = 0.5f;
/// <summary>
/// At most 20% memory overhead.
/// </summary>
- public static float DEFAULT = 0.2f;
+ public static readonly float DEFAULT = 0.2f;
/// <summary>
/// No memory overhead at all, but the returned implementation may be slow.
/// </summary>
- public static float COMPACT = 0f;
+ public static readonly float COMPACT = 0f;
/// <summary>
/// Default amount of memory to use for bulk operations.
/// </summary>
- public static int DEFAULT_BUFFER_SIZE = 1024; // 1K
+ public static readonly int DEFAULT_BUFFER_SIZE = 1024; // 1K
- public static string CODEC_NAME = "PackedInts";
- public static int VERSION_START = 0; // PackedInts were long-aligned
- public static int VERSION_BYTE_ALIGNED = 1;
- public static int VERSION_CURRENT = VERSION_BYTE_ALIGNED;
+ public static readonly string CODEC_NAME = "PackedInts";
Review Comment:
Will accept this because it was this way in [Lucene 4.8.0](https://github.com/apache/lucene/blob/releases/lucene-solr/4.8.0/lucene/core/src/java/org/apache/lucene/util/packed/PackedInts.java#L66), but do note that this was supposed to be part of #662 (it should be const).
##########
src/Lucene.Net/Util/Packed/PackedInts.cs:
##########
@@ -46,32 +46,32 @@ public static class PackedInt32s // LUCENENET specific: CA1052 Static holder typ
/// <summary>
/// At most 700% memory overhead, always select a direct implementation.
/// </summary>
- public static float FASTEST = 7f;
+ public static readonly float FASTEST = 7f;
/// <summary>
/// At most 50% memory overhead, always select a reasonably fast implementation.
/// </summary>
- public static float FAST = 0.5f;
+ public static readonly float FAST = 0.5f;
/// <summary>
/// At most 20% memory overhead.
/// </summary>
- public static float DEFAULT = 0.2f;
+ public static readonly float DEFAULT = 0.2f;
/// <summary>
/// No memory overhead at all, but the returned implementation may be slow.
/// </summary>
- public static float COMPACT = 0f;
+ public static readonly float COMPACT = 0f;
/// <summary>
/// Default amount of memory to use for bulk operations.
/// </summary>
- public static int DEFAULT_BUFFER_SIZE = 1024; // 1K
+ public static readonly int DEFAULT_BUFFER_SIZE = 1024; // 1K
- public static string CODEC_NAME = "PackedInts";
- public static int VERSION_START = 0; // PackedInts were long-aligned
- public static int VERSION_BYTE_ALIGNED = 1;
- public static int VERSION_CURRENT = VERSION_BYTE_ALIGNED;
+ public static readonly string CODEC_NAME = "PackedInts";
+ public static readonly int VERSION_START = 0; // PackedInts were long-aligned
+ public static readonly int VERSION_BYTE_ALIGNED = 1;
+ public static readonly int VERSION_CURRENT = VERSION_BYTE_ALIGNED;
Review Comment:
Will accept this because it was this way in [Lucene 4.8.0](https://github.com/apache/lucene/blob/releases/lucene-solr/4.8.0/lucene/core/src/java/org/apache/lucene/util/packed/PackedInts.java#L69), but do note that this was supposed to be part of #662 (it should be const).
##########
src/Lucene.Net.QueryParser/Flexible/Standard/Parser/StandardSyntaxParserConstants.cs:
##########
@@ -103,7 +103,7 @@ public static class LexicalToken
public static class StandardSyntaxParserConstants
{
/// <summary>Literal token values.</summary>
- public static string[] TokenImage = new string[] {
+ internal static readonly string[] TOKEN_IMAGE = new string[] {
Review Comment:
Please revert. See the comment in Classic QueryParser.
##########
src/Lucene.Net.QueryParser/Classic/QueryParserConstants.cs:
##########
@@ -108,7 +108,7 @@ public static class LexicalToken
public static class QueryParserConstants
{
/// <summary>Literal token values. </summary>
- public static string[] TokenImage = new string[] {
+ internal static readonly string[] TOKEN_IMAGE = new string[] {
Review Comment:
Please revert. See the comment in Classic QueryParser.
##########
src/Lucene.Net.QueryParser/Classic/QueryParser.cs:
##########
@@ -889,7 +889,7 @@ public virtual ParseException GenerateParseException()
{
exptokseq[i] = jj_expentries[i];
}
- return new ParseException(Token, exptokseq, QueryParserConstants.TokenImage);
+ return new ParseException(Token, exptokseq, QueryParserConstants.TOKEN_IMAGE);
Review Comment:
For now, let's revert this change. It seems silly to have a public `QueryParserConstants` class with an internal `TOKEN_IMAGE`. The QueryParsers contain generated code that I would rather leave the project alone until we get a chance to port or locate an adequate code generator to do the same job.
Let's leave this in the scan until we figure out how to deal with it. At the very least, this PR should contain no breaking public API changes.
##########
src/Lucene.Net/Util/Packed/PackedInts.cs:
##########
@@ -46,32 +46,32 @@ public static class PackedInt32s // LUCENENET specific: CA1052 Static holder typ
/// <summary>
/// At most 700% memory overhead, always select a direct implementation.
/// </summary>
- public static float FASTEST = 7f;
+ public static readonly float FASTEST = 7f;
/// <summary>
/// At most 50% memory overhead, always select a reasonably fast implementation.
/// </summary>
- public static float FAST = 0.5f;
+ public static readonly float FAST = 0.5f;
/// <summary>
/// At most 20% memory overhead.
/// </summary>
- public static float DEFAULT = 0.2f;
+ public static readonly float DEFAULT = 0.2f;
/// <summary>
/// No memory overhead at all, but the returned implementation may be slow.
/// </summary>
- public static float COMPACT = 0f;
+ public static readonly float COMPACT = 0f;
/// <summary>
/// Default amount of memory to use for bulk operations.
/// </summary>
- public static int DEFAULT_BUFFER_SIZE = 1024; // 1K
+ public static readonly int DEFAULT_BUFFER_SIZE = 1024; // 1K
- public static string CODEC_NAME = "PackedInts";
- public static int VERSION_START = 0; // PackedInts were long-aligned
- public static int VERSION_BYTE_ALIGNED = 1;
- public static int VERSION_CURRENT = VERSION_BYTE_ALIGNED;
+ public static readonly string CODEC_NAME = "PackedInts";
+ public static readonly int VERSION_START = 0; // PackedInts were long-aligned
Review Comment:
Will accept this because it was this way in [Lucene 4.8.0](https://github.com/apache/lucene/blob/releases/lucene-solr/4.8.0/lucene/core/src/java/org/apache/lucene/util/packed/PackedInts.java#L67), but do note that this was supposed to be part of #662 (these should be const).
##########
src/Lucene.Net/Util/Packed/PackedInts.cs:
##########
@@ -46,32 +46,32 @@ public static class PackedInt32s // LUCENENET specific: CA1052 Static holder typ
/// <summary>
/// At most 700% memory overhead, always select a direct implementation.
/// </summary>
- public static float FASTEST = 7f;
+ public static readonly float FASTEST = 7f;
/// <summary>
/// At most 50% memory overhead, always select a reasonably fast implementation.
/// </summary>
- public static float FAST = 0.5f;
+ public static readonly float FAST = 0.5f;
/// <summary>
/// At most 20% memory overhead.
/// </summary>
- public static float DEFAULT = 0.2f;
+ public static readonly float DEFAULT = 0.2f;
/// <summary>
/// No memory overhead at all, but the returned implementation may be slow.
/// </summary>
- public static float COMPACT = 0f;
+ public static readonly float COMPACT = 0f;
/// <summary>
/// Default amount of memory to use for bulk operations.
/// </summary>
- public static int DEFAULT_BUFFER_SIZE = 1024; // 1K
+ public static readonly int DEFAULT_BUFFER_SIZE = 1024; // 1K
- public static string CODEC_NAME = "PackedInts";
- public static int VERSION_START = 0; // PackedInts were long-aligned
- public static int VERSION_BYTE_ALIGNED = 1;
- public static int VERSION_CURRENT = VERSION_BYTE_ALIGNED;
+ public static readonly string CODEC_NAME = "PackedInts";
+ public static readonly int VERSION_START = 0; // PackedInts were long-aligned
+ public static readonly int VERSION_BYTE_ALIGNED = 1;
Review Comment:
Will accept this because it was this way in [Lucene 4.8.0](https://github.com/apache/lucene/blob/releases/lucene-solr/4.8.0/lucene/core/src/java/org/apache/lucene/util/packed/PackedInts.java#L68), but do note that this was supposed to be part of #662 (it should be const).
##########
src/Lucene.Net.QueryParser/Xml/CoreParser.cs:
##########
@@ -35,7 +35,7 @@ public class CoreParser : IQueryBuilder
protected QueryBuilderFactory m_queryFactory;
protected FilterBuilderFactory m_filterFactory;
//Controls the max size of the LRU cache used for QueryFilter objects parsed.
- public static int maxNumCachedFilters = 20;
+ const int maxNumCachedFilters = 20;
Review Comment:
This was public in [Lucene 4.8.1](https://github.com/apache/lucene/blob/releases/lucene-solr/4.8.1/lucene/queryparser/src/java/org/apache/lucene/queryparser/xml/CoreParser.java#L41). Also, given the naming convention and comment above it, I suspect that this is intended to be settable by end users. Let's revert this and add the following attribute.
`[SuppressMessage("Usage", "CA2211:Non-constant fields should not be visible", Justification = "Lucene may have intended for this to be a user setting")]`
##########
src/Lucene.Net.QueryParser/Surround/Parser/ParseException.cs:
##########
@@ -197,7 +197,7 @@ public override void GetObjectData(SerializationInfo info, StreamingContext cont
/// <summary>
/// The end of line string for this machine.
/// </summary>
- protected static string eol = Environment.NewLine;
+ protected static readonly string eol = Environment.NewLine;
Review Comment:
Please add a comment to indicate we have intentionally deviated from [Lucene 4.8.1](https://github.com/apache/lucene/blob/releases/lucene-solr/4.8.1/lucene/queryparser/src/java/org/apache/lucene/queryparser/surround/parser/ParseException.java#L134) on this line.
`// LUCENENET: Marked readonly`
##########
src/Lucene.Net.QueryParser/Surround/Parser/QueryParserConstants.cs:
##########
@@ -88,7 +88,7 @@ public static class LexicalToken
public static class QueryParserConstants
{
/// <summary>Literal token values. </summary>
- public static string[] TokenImage = new string[] {
+ internal static readonly string[] TOKEN_IMAGE = new string[] {
Review Comment:
Please revert. See the comment in Classic QueryParser.
##########
src/Lucene.Net.QueryParser/Surround/Parser/QueryParser.cs:
##########
@@ -865,7 +865,7 @@ public virtual ParseException GenerateParseException()
{
exptokseq[i] = jj_expentries[i];
}
- return new ParseException(Token, exptokseq, QueryParserConstants.TokenImage);
+ return new ParseException(Token, exptokseq, QueryParserConstants.TOKEN_IMAGE);
Review Comment:
Please revert. See the comment in Classic QueryParser.
##########
src/Lucene.Net/Search/ConstantScoreAutoRewrite.cs:
##########
@@ -50,13 +50,13 @@ public class ConstantScoreAutoRewrite : TermCollectingRewrite<BooleanQuery>
/// doc Wikipedia index. With more than 350 terms in the
/// query, the filter method is fastest:
/// </summary>
- public static int DEFAULT_TERM_COUNT_CUTOFF = 350;
+ const int DEFAULT_TERM_COUNT_CUTOFF = 350;
/// <summary>
/// If the query will hit more than 1 in 1000 of the docs
/// in the index (0.1%), the filter method is fastest:
/// </summary>
- public static double DEFAULT_DOC_COUNT_PERCENT = 0.1;
+ const double DEFAULT_DOC_COUNT_PERCENT = 0.1;
Review Comment:
This was `public` in [Lucene 4.8.0](https://github.com/apache/lucene/blob/releases/lucene-solr/4.8.0/lucene/core/src/java/org/apache/lucene/search/ConstantScoreAutoRewrite.java#L43). Please change back to public, but leave `const`, as that is the right choice here.
##########
src/Lucene.Net/Search/ConstantScoreAutoRewrite.cs:
##########
@@ -50,13 +50,13 @@ public class ConstantScoreAutoRewrite : TermCollectingRewrite<BooleanQuery>
/// doc Wikipedia index. With more than 350 terms in the
/// query, the filter method is fastest:
/// </summary>
- public static int DEFAULT_TERM_COUNT_CUTOFF = 350;
+ const int DEFAULT_TERM_COUNT_CUTOFF = 350;
Review Comment:
This was `public` in [Lucene 4.8.0](https://github.com/apache/lucene/blob/releases/lucene-solr/4.8.0/lucene/core/src/java/org/apache/lucene/search/ConstantScoreAutoRewrite.java#L39). Please change back to public, but leave `const`, as that is the right choice here.
##########
src/Lucene.Net/Util/Automaton/Automaton.cs:
##########
@@ -78,11 +78,11 @@ public class Automaton // LUCENENET specific: Not implementing ICloneable per Mi
/// the most generally efficient algorithms that exist.
/// </summary>
/// <seealso cref="SetMinimization(int)"/>
- public const int MINIMIZE_HOPCROFT = 2;
+ private const int MINIMIZE_HOPCROFT = 2;
/// <summary>
/// Selects minimization algorithm (default: <c>MINIMIZE_HOPCROFT</c>). </summary>
- internal static int minimization = MINIMIZE_HOPCROFT;
+ private static int _minimization = MINIMIZE_HOPCROFT;
Review Comment:
Please revert the name to `minimization` as it was in [Lucene 4.8.0](https://github.com/apache/lucene/blob/releases/lucene-solr/4.8.0/lucene/core/src/java/org/apache/lucene/util/automaton/Automaton.java#L88), but since it compiles with `private` it is fine to keep it. Please add the following comment after the line to indicate we deviated from Lucene on purpose.
`// LUCENENET: Changed from internal to private`
##########
src/Lucene.Net/Util/Automaton/Automaton.cs:
##########
@@ -78,11 +78,11 @@ public class Automaton // LUCENENET specific: Not implementing ICloneable per Mi
/// the most generally efficient algorithms that exist.
/// </summary>
/// <seealso cref="SetMinimization(int)"/>
- public const int MINIMIZE_HOPCROFT = 2;
+ private const int MINIMIZE_HOPCROFT = 2;
Review Comment:
Please revert. This was `public` in [Lucene 4.8.0](https://github.com/apache/lucene/blob/releases/lucene-solr/4.8.0/lucene/core/src/java/org/apache/lucene/util/automaton/Automaton.java#L85). `const` is the right decision here.
##########
src/Lucene.Net/Search/FieldCache.cs:
##########
@@ -702,7 +702,7 @@ public interface IDoubleParser : IParser
/// <summary>
/// Expert: The cache used internally by sorting and range query classes.
/// </summary>
- public static IFieldCache DEFAULT = new FieldCacheImpl();
+ internal static readonly IFieldCache DEFAULT = new FieldCacheImpl();
Review Comment:
This was public and not final in [Lucene 4.8.0](https://github.com/apache/lucene/blob/releases/lucene-solr/4.8.0/lucene/core/src/java/org/apache/lucene/search/FieldCache.java#L214). Please change back to `public`, remove `readonly` and add the following attribute to suppress the warning.
`[SuppressMessage("Usage", "CA2211:Non-constant fields should not be visible", Justification = "Lucene may have intended for this to be a user setting")]`
##########
src/Lucene.Net/Codecs/Lucene41/Lucene41PostingsFormat.cs:
##########
@@ -372,7 +372,7 @@ public sealed class Lucene41PostingsFormat : PostingsFormat
/// a single packed block.
/// </summary>
// NOTE: must be multiple of 64 because of PackedInts long-aligned encoding/decoding
- public static int BLOCK_SIZE = 128;
+ public static readonly int BLOCK_SIZE = 128;
Review Comment:
Will accept this because it was this way in [Lucene 4.8.0](https://github.com/apache/lucene/blob/releases/lucene-solr/4.8.0/lucene/core/src/java/org/apache/lucene/codecs/lucene41/Lucene41PostingsFormat.java#L388), but do note that this was supposed to be part of #662 (it should be const).
##########
src/Lucene.Net/Index/IndexFileDeleter.cs:
##########
@@ -113,7 +113,7 @@ internal sealed class IndexFileDeleter : IDisposable
/// Change to true to see details of reference counts when
/// infoStream is enabled
/// </summary>
- public static bool VERBOSE_REF_COUNTS = false;
+ public static readonly bool VERBOSE_REF_COUNTS = false;
Review Comment:
Clearly by the doc comment, this was intended to be a user setting. Please revert and add the following attribute to suppress this.
`[SuppressMessage("Usage", "CA2211:Non-constant fields should not be visible", Justification = "Lucene intended for this to be a user setting")]`
##########
src/Lucene.Net/Store/Lock.cs:
##########
@@ -50,7 +50,7 @@ public abstract class Lock : IDisposable
/// Pass this value to <see cref="Obtain(long)"/> to try
/// forever to obtain the lock.
/// </summary>
- public const long LOCK_OBTAIN_WAIT_FOREVER = -1;
+ const long LOCK_OBTAIN_WAIT_FOREVER = -1;
Review Comment:
`LOCK_OBTAIN_WAIT_FOREVER` was not being flagged by the scan and was marked `public` in [Lucene 4.8.0](https://github.com/apache/lucene/blob/releases/lucene-solr/4.8.0/lucene/core/src/java/org/apache/lucene/store/Lock.java#L46). Please revert.
`LOCK_POLL_INTERVAL` was what the scan was complaining about. `LOCK_POLL_INTERVAL` was not marked `final` in [Lucene 4.8.0](https://github.com/apache/lucene/blob/releases/lucene-solr/4.8.0/lucene/core/src/java/org/apache/lucene/store/Lock.java#L42), so let's add the following attribute to it.
`[SuppressMessage("Usage", "CA2211:Non-constant fields should not be visible", Justification = "Lucene may have intended for this to be a user setting")]`
##########
src/Lucene.Net/Util/Automaton/Automaton.cs:
##########
@@ -109,13 +109,13 @@ public class Automaton // LUCENENET specific: Not implementing ICloneable per Mi
/// <summary>
/// Minimize always flag. </summary>
- internal static bool minimize_always = false;
+ private static bool _minimizeAlways = false;
/// <summary>
/// Selects whether operations may modify the input automata (default:
/// <c>false</c>).
/// </summary>
- internal static bool allow_mutation = false;
+ private static bool allow_mutation = false;
Review Comment:
Please add the following comment after the line to indicate we deviated from Lucene on purpose.
`// LUCENENET: Changed from internal to private`
##########
src/Lucene.Net/Util/Automaton/Automaton.cs:
##########
@@ -109,13 +109,13 @@ public class Automaton // LUCENENET specific: Not implementing ICloneable per Mi
/// <summary>
/// Minimize always flag. </summary>
- internal static bool minimize_always = false;
+ private static bool _minimizeAlways = false;
Review Comment:
Please revert the name to `minimize_always` as it was in [Lucene 4.8.0](https://github.com/apache/lucene/blob/releases/lucene-solr/4.8.0/lucene/core/src/java/org/apache/lucene/util/automaton/Automaton.java#L88), but since it compiles with `private` it is fine to keep it. Please add the following comment after the line to indicate we deviated from Lucene on purpose.
`// LUCENENET: Changed from internal to private`
##########
src/Lucene.Net/Search/Similarities/BasicModelP.cs:
##########
@@ -33,7 +33,7 @@ public class BasicModelP : BasicModel
{
/// <summary>
/// <c>log2(Math.E)</c>, precomputed. </summary>
- protected internal static double LOG2_E = Log2(Math.E);
+ private static readonly double LOG2_E = Log2(Math.E);
Review Comment:
This was `protected` in [Lucene 4.8.0](https://github.com/apache/lucene/blob/releases/lucene-solr/4.8.0/lucene/core/src/java/org/apache/lucene/search/similarities/BasicModelP.java#L32). Please change back to `protected`, although there is no need for the `internal` modifier here. Keep the `readonly` modifier and add the following comment at the end of the line.
`protected readonly static double LOG2_E = Log2(Math.E); // LUCENENET: Marked readonly`
##########
src/Lucene.Net.Analysis.Common/Analysis/En/KStemData1.cs:
##########
@@ -49,7 +49,7 @@ private KStemData1()
{
}
// KStemData1 ... KStemData8 are created from "head_word_list.txt"
- internal static string[] data = new string[] {
+ internal static readonly string[] data = new string[] {
Review Comment:
Please add a comment to indicate we have intentionally deviated from [Lucene 4.8.1](https://github.com/apache/lucene/blob/releases/lucene-solr/4.8.1/lucene/analysis/common/src/java/org/apache/lucene/analysis/en/KStemData1.java#L48) on this line.
##########
src/Lucene.Net/Util/Packed/PackedInts.cs:
##########
@@ -46,32 +46,32 @@ public static class PackedInt32s // LUCENENET specific: CA1052 Static holder typ
/// <summary>
/// At most 700% memory overhead, always select a direct implementation.
/// </summary>
- public static float FASTEST = 7f;
+ public static readonly float FASTEST = 7f;
/// <summary>
/// At most 50% memory overhead, always select a reasonably fast implementation.
/// </summary>
- public static float FAST = 0.5f;
+ public static readonly float FAST = 0.5f;
/// <summary>
/// At most 20% memory overhead.
/// </summary>
- public static float DEFAULT = 0.2f;
+ public static readonly float DEFAULT = 0.2f;
Review Comment:
Will accept this because it was this way in [Lucene 4.8.0](https://github.com/apache/lucene/blob/releases/lucene-solr/4.8.0/lucene/core/src/java/org/apache/lucene/util/packed/PackedInts.java#L54), but do note that this was supposed to be part of #662 (it should be const).
##########
src/Lucene.Net/Util/Automaton/State.cs:
##########
@@ -58,7 +58,7 @@ public class State : IComparable<State>
internal int number;
internal int id;
- internal static int next_id;
+ private static int next_id;
Review Comment:
Please add the following comment after the line to indicate we deviated from Lucene on purpose.
`// LUCENENET: Changed from internal to private`
##########
src/Lucene.Net/Util/Automaton/UTF32ToUTF8.cs:
##########
@@ -40,7 +40,7 @@ public sealed class UTF32ToUTF8
private static readonly int[] endCodes = new int[] { 127, 2047, 65535, 1114111 };
- internal static int[] MASKS = LoadMasks();
+ private static int[] MASKS = LoadMasks();
Review Comment:
Please add the following comment after the line to indicate we deviated from Lucene on purpose.
`// LUCENENET: Changed from internal to private`
##########
src/Lucene.Net/Util/Packed/PackedInts.cs:
##########
@@ -46,32 +46,32 @@ public static class PackedInt32s // LUCENENET specific: CA1052 Static holder typ
/// <summary>
/// At most 700% memory overhead, always select a direct implementation.
/// </summary>
- public static float FASTEST = 7f;
+ public static readonly float FASTEST = 7f;
Review Comment:
Will accept this because it was this way in [Lucene 4.8.0](https://github.com/apache/lucene/blob/releases/lucene-solr/4.8.0/lucene/core/src/java/org/apache/lucene/util/packed/PackedInts.java#L44), but do note that this was supposed to be part of #662 (it should be const).
##########
src/Lucene.Net/Util/Packed/PackedInts.cs:
##########
@@ -46,32 +46,32 @@ public static class PackedInt32s // LUCENENET specific: CA1052 Static holder typ
/// <summary>
/// At most 700% memory overhead, always select a direct implementation.
/// </summary>
- public static float FASTEST = 7f;
+ public static readonly float FASTEST = 7f;
/// <summary>
/// At most 50% memory overhead, always select a reasonably fast implementation.
/// </summary>
- public static float FAST = 0.5f;
+ public static readonly float FAST = 0.5f;
/// <summary>
/// At most 20% memory overhead.
/// </summary>
- public static float DEFAULT = 0.2f;
+ public static readonly float DEFAULT = 0.2f;
/// <summary>
/// No memory overhead at all, but the returned implementation may be slow.
/// </summary>
- public static float COMPACT = 0f;
+ public static readonly float COMPACT = 0f;
/// <summary>
/// Default amount of memory to use for bulk operations.
/// </summary>
- public static int DEFAULT_BUFFER_SIZE = 1024; // 1K
+ public static readonly int DEFAULT_BUFFER_SIZE = 1024; // 1K
Review Comment:
Will accept this because it was this way in [Lucene 4.8.0](https://github.com/apache/lucene/blob/releases/lucene-solr/4.8.0/lucene/core/src/java/org/apache/lucene/util/packed/PackedInts.java#L64), but do note that this was supposed to be part of #662 (it should be const).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@lucenenet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [lucenenet] sachdevlaksh closed pull request #729: #679 Added sonar suggestions
Posted by GitBox <gi...@apache.org>.
sachdevlaksh closed pull request #729: #679 Added sonar suggestions
URL: https://github.com/apache/lucenenet/pull/729
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@lucenenet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [lucenenet] NightOwl888 commented on a diff in pull request #729: #679 Added sonar suggestions
Posted by GitBox <gi...@apache.org>.
NightOwl888 commented on code in PR #729:
URL: https://github.com/apache/lucenenet/pull/729#discussion_r1009003373
##########
src/Lucene.Net.Analysis.Common/Analysis/En/KStemData1.cs:
##########
@@ -49,7 +49,7 @@ private KStemData1()
{
}
// KStemData1 ... KStemData8 are created from "head_word_list.txt"
- internal static string[] data = new string[] {
+ internal static readonly string[] data = new string[] {
Review Comment:
Please add a comment to indicate we have intentionally deviated from [Lucene 4.8.1](https://github.com/apache/lucene/blob/releases/lucene-solr/4.8.1/lucene/analysis/common/src/java/org/apache/lucene/analysis/en/KStemData1.java#L48) on this line.
`// LUCENENET: Marked readonly`
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@lucenenet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org