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
Make sure that that every function in Phobos has an example: std.{algorithm,array,ascii,base64} #5581
Conversation
Thanks for your pull request, @wilzbach! Bugzilla referencesYour PR doesn't reference any Bugzilla issue. If your PR contains non-trivial changes, please reference a Bugzilla issue or create a manual changelog. |
6f563e9
to
a7446db
Compare
a7446db
to
f164ee4
Compare
@@ -120,7 +120,7 @@ exception_check="-std.concurrency,-std.net.curl,-std.parallelism,-std.range,-std | |||
; Checks for poor placement of function attributes | |||
function_attribute_check="-std.algorithm.iteration,-std.concurrency,-std.conv,-std.datetime.interval,-std.exception,-std.functional,-std.net.curl,-std.numeric,-std.parallelism,-std.random,-std.range,-std.range.primitives,-std.socket,-std.traits,-std.typecons,-std.uni" | |||
; Check for public declarations without a documented unittest | |||
has_public_example="-etc.c.curl,-etc.c.sqlite3,-etc.c.zlib,-std.algorithm.comparison,-std.algorithm.mutation,-std.array,-std.ascii,-std.base64,-std.bitmanip,-std.complex,-std.concurrency,-std.container.array,-std.container.dlist,-std.container.rbtree,-std.container.slist,-std.conv,-std.csv,-std.datetime,-std.datetime.date,-std.datetime.interval,-std.datetime.stopwatch,-std.datetime.systime,-std.datetime.timezone,-std.demangle,-std.digest.digest,-std.digest.hmac,-std.digest.murmurhash,-std.digest.sha,-std.encoding,-std.exception,-std.experimental.allocator,-std.experimental.allocator.building_blocks.allocator_list,-std.experimental.allocator.building_blocks.bitmapped_block,-std.experimental.allocator.building_blocks.fallback_allocator,-std.experimental.allocator.building_blocks.free_list,-std.experimental.allocator.building_blocks.free_tree,-std.experimental.allocator.building_blocks.null_allocator,-std.experimental.allocator.building_blocks.stats_collector,-std.experimental.allocator.common,-std.experimental.allocator.mmap_allocator,-std.experimental.allocator.typed,-std.experimental.checkedint,-std.experimental.logger.core,-std.experimental.logger.filelogger,-std.experimental.logger.multilogger,-std.experimental.typecons,-std.file,-std.format,-std.getopt,-std.internal.math.biguintcore,-std.internal.math.biguintnoasm,-std.internal.math.errorfunction,-std.internal.math.gammafunction,-std.internal.scopebuffer,-std.internal.test.dummyrange,-std.json,-std.math,-std.mathspecial,-std.mmfile,-std.net.curl,-std.net.isemail,-std.numeric,-std.outbuffer,-std.parallelism,-std.path,-std.process,-std.random,-std.range,-std.range.interfaces,-std.range.primitives,-std.regex,-std.regex.internal.ir,-std.socket,-std.stdio,-std.string,-std.traits,-std.typecons,-std.uni,-std.uri,-std.utf,-std.uuid,-std.variant,-std.xml,-std.zip,-std.zlib" | |||
has_public_example="-etc.c.curl,-etc.c.sqlite3,-etc.c.zlib,-std.bitmanip,-std.complex,-std.concurrency,-std.container.array,-std.container.dlist,-std.container.rbtree,-std.container.slist,-std.conv,-std.csv,-std.datetime,-std.datetime.date,-std.datetime.interval,-std.datetime.stopwatch,-std.datetime.systime,-std.datetime.timezone,-std.demangle,-std.digest.digest,-std.digest.hmac,-std.digest.murmurhash,-std.digest.sha,-std.encoding,-std.exception,-std.experimental.allocator,-std.experimental.allocator.building_blocks.allocator_list,-std.experimental.allocator.building_blocks.bitmapped_block,-std.experimental.allocator.building_blocks.fallback_allocator,-std.experimental.allocator.building_blocks.free_list,-std.experimental.allocator.building_blocks.free_tree,-std.experimental.allocator.building_blocks.null_allocator,-std.experimental.allocator.building_blocks.stats_collector,-std.experimental.allocator.common,-std.experimental.allocator.mmap_allocator,-std.experimental.allocator.typed,-std.experimental.checkedint,-std.experimental.logger.core,-std.experimental.logger.filelogger,-std.experimental.logger.multilogger,-std.experimental.typecons,-std.file,-std.format,-std.getopt,-std.internal.math.biguintcore,-std.internal.math.biguintnoasm,-std.internal.math.errorfunction,-std.internal.math.gammafunction,-std.internal.scopebuffer,-std.internal.test.dummyrange,-std.json,-std.math,-std.mathspecial,-std.mmfile,-std.net.curl,-std.net.isemail,-std.numeric,-std.outbuffer,-std.parallelism,-std.path,-std.process,-std.random,-std.range,-std.range.interfaces,-std.range.primitives,-std.regex,-std.regex.internal.ir,-std.socket,-std.stdio,-std.string,-std.traits,-std.typecons,-std.uni,-std.uri,-std.utf,-std.uuid,-std.variant,-std.xml,-std.zip,-std.zlib" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wish we could generate this list manually automatically.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
FYI the list was generated automatically: #5501 - just reducing it isn't automated :/
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Whoops, I just realized I wrote manually by mistake. At least the generating part works so we're not too bad off.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
std/array.d
Outdated
unittest | ||
{ | ||
auto w = appender!string; | ||
// pre-allocate at least 10 elements (this avoids costly reallocations) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nitpick: pre-allocate at least 10 elements
-> pre-allocate space for at least 10 elements
std/array.d
Outdated
/// | ||
@safe unittest | ||
{ | ||
assert("Hello D".array == "Hello D"d); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just in case people don't know who are reading this, I would add
static assert(isRandomAccessRange!string == false);
here, and then
static assert(isRandomAccessRange!dstring == true);
at the end
std/array.d
Outdated
@@ -588,6 +595,7 @@ if (isDynamicArray!T && allSatisfy!(isIntegral, I)) | |||
return arrayAllocImpl!(true, T, ST)(sizes); | |||
} | |||
|
|||
/// | |||
@safe pure nothrow unittest |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This example is rather confusing, and seems to be testing edge cases. Can something more simple be used here?
std/base64.d
Outdated
|
||
// pre-defined: alias Base64 = Base64Impl!('+', '/'); | ||
ubyte[] emptyArr; | ||
assert(Base64.encode(emptyArr) == ""); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's avoid aligned equals signs. See the Class/Struct Field Declarations
section here https://dlang.org/dstyle.html
f164ee4
to
0fb288f
Compare
This was done as it was the existing style, but fair enough ;-) |
0fb288f
to
54c47a7
Compare
@safe unittest | ||
{ | ||
import std.stdio; | ||
import std.algorithm.sorting : partition; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why do you import "partition"?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No particular reason except for copy/pasting from the test below :/
From @WalterBright's call to action:
The check
has_public_example
has been there for quite some time, so I thought I go ahead and set a good example.@ everyone there are many checks that would be worthwhile to reduce one module at a time.
Apart from
has_public_example
, there'sproperly_documented_public_functions
(checks that every function has "Params" + "Returns", part Walter's calls to action: 2016, 2015,could_be_immutable_check
or find TODO checks yourself in the .dscanner.ini.